Skip to content

Commit 585b5da

Browse files
committed
refcount: make refcounting conform to aliasing rules
Strict aliasing rules dictate that for most data types, you are not allowed to cast them to another data type and then access the casted pointers. While this works just fine for most compilers, technically we end up in undefined behaviour when we hurt that rule. Our current refcounting code makes heavy use of casting and thus violates that rule. While we didn't have any problems with that code, Travis started spitting out a lot of warnings due to a change in their toolchain. In the refcounting case, the code is also easy to fix: as all refcounting-statements are actually macros, we can just access the `rc` field directly instead of casting. There are two outliers in our code where that doesn't work. Both the `git_diff` and `git_patch` structures have specializations for generated and parsed diffs/patches, which directly inherit from them. Because of that, the refcounting code is only part of the base structure and not of the children themselves. We can help that by instead passing their base into `GIT_REFCOUNT_INC`, though.
1 parent fd1492e commit 585b5da

File tree

6 files changed

+10
-10
lines changed

6 files changed

+10
-10
lines changed

src/diff_generate.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ static git_diff_generated *diff_generated_alloc(
411411
if ((diff = git__calloc(1, sizeof(git_diff_generated))) == NULL)
412412
return NULL;
413413

414-
GIT_REFCOUNT_INC(diff);
414+
GIT_REFCOUNT_INC(&diff->base);
415415
diff->base.type = GIT_DIFF_TYPE_GENERATED;
416416
diff->base.repo = repo;
417417
diff->base.old_src = old_iter->type;

src/diff_parse.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ static git_diff_parsed *diff_parsed_alloc(void)
3636
if ((diff = git__calloc(1, sizeof(git_diff_parsed))) == NULL)
3737
return NULL;
3838

39-
GIT_REFCOUNT_INC(diff);
39+
GIT_REFCOUNT_INC(&diff->base);
4040
diff->base.type = GIT_DIFF_TYPE_PARSED;
4141
diff->base.strcomp = git__strcmp;
4242
diff->base.strncomp = git__strncmp;

src/patch_generate.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ static int patch_generated_alloc_from_diff(
139139

140140
if (!(error = patch_generated_init(patch, diff, delta_index))) {
141141
patch->flags |= GIT_PATCH_GENERATED_ALLOCATED;
142-
GIT_REFCOUNT_INC(patch);
142+
GIT_REFCOUNT_INC(&patch->base);
143143
} else {
144144
git__free(patch);
145145
patch = NULL;

src/patch_parse.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,7 @@ int git_patch_parse(
10841084
patch->base.diff_opts.new_prefix = patch->new_prefix;
10851085
patch->base.diff_opts.flags |= GIT_DIFF_SHOW_BINARY;
10861086

1087-
GIT_REFCOUNT_INC(patch);
1087+
GIT_REFCOUNT_INC(&patch->base);
10881088
*out = &patch->base;
10891089

10901090
done:

src/repository.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
#include <ctype.h>
1111

1212
#include "git2/object.h"
13-
#include "git2/refdb.h"
1413
#include "git2/sys/repository.h"
1514

1615
#include "common.h"
@@ -25,6 +24,7 @@
2524
#include "refs.h"
2625
#include "filter.h"
2726
#include "odb.h"
27+
#include "refdb.h"
2828
#include "remote.h"
2929
#include "merge.h"
3030
#include "diff_driver.h"

src/util.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,22 +297,22 @@ typedef struct {
297297
typedef void (*git_refcount_freeptr)(void *r);
298298

299299
#define GIT_REFCOUNT_INC(r) { \
300-
git_atomic_inc(&((git_refcount *)(r))->refcount); \
300+
git_atomic_inc(&(r)->rc.refcount); \
301301
}
302302

303303
#define GIT_REFCOUNT_DEC(_r, do_free) { \
304-
git_refcount *r = (git_refcount *)(_r); \
304+
git_refcount *r = &(_r)->rc; \
305305
int val = git_atomic_dec(&r->refcount); \
306306
if (val <= 0 && r->owner == NULL) { do_free(_r); } \
307307
}
308308

309309
#define GIT_REFCOUNT_OWN(r, o) { \
310-
((git_refcount *)(r))->owner = o; \
310+
(r)->rc.owner = o; \
311311
}
312312

313-
#define GIT_REFCOUNT_OWNER(r) (((git_refcount *)(r))->owner)
313+
#define GIT_REFCOUNT_OWNER(r) ((r)->rc.owner)
314314

315-
#define GIT_REFCOUNT_VAL(r) git_atomic_get(&((git_refcount *)(r))->refcount)
315+
#define GIT_REFCOUNT_VAL(r) git_atomic_get((r)->rc.refcount)
316316

317317

318318
static signed char from_hex[] = {

0 commit comments

Comments
 (0)