Commit 585b5dacc7f440a163c20117cfa35fb714a7ba7b
2017-11-18T15:43:11
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 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104
diff --git a/src/diff_generate.c b/src/diff_generate.c
index 6436ab9..6b3fa4f 100644
--- a/src/diff_generate.c
+++ b/src/diff_generate.c
@@ -411,7 +411,7 @@ static git_diff_generated *diff_generated_alloc(
if ((diff = git__calloc(1, sizeof(git_diff_generated))) == NULL)
return NULL;
- GIT_REFCOUNT_INC(diff);
+ GIT_REFCOUNT_INC(&diff->base);
diff->base.type = GIT_DIFF_TYPE_GENERATED;
diff->base.repo = repo;
diff->base.old_src = old_iter->type;
diff --git a/src/diff_parse.c b/src/diff_parse.c
index 2838314..59fd861 100644
--- a/src/diff_parse.c
+++ b/src/diff_parse.c
@@ -36,7 +36,7 @@ static git_diff_parsed *diff_parsed_alloc(void)
if ((diff = git__calloc(1, sizeof(git_diff_parsed))) == NULL)
return NULL;
- GIT_REFCOUNT_INC(diff);
+ GIT_REFCOUNT_INC(&diff->base);
diff->base.type = GIT_DIFF_TYPE_PARSED;
diff->base.strcomp = git__strcmp;
diff->base.strncomp = git__strncmp;
diff --git a/src/patch_generate.c b/src/patch_generate.c
index 3995c56..29cda8b 100644
--- a/src/patch_generate.c
+++ b/src/patch_generate.c
@@ -139,7 +139,7 @@ static int patch_generated_alloc_from_diff(
if (!(error = patch_generated_init(patch, diff, delta_index))) {
patch->flags |= GIT_PATCH_GENERATED_ALLOCATED;
- GIT_REFCOUNT_INC(patch);
+ GIT_REFCOUNT_INC(&patch->base);
} else {
git__free(patch);
patch = NULL;
diff --git a/src/patch_parse.c b/src/patch_parse.c
index fee6afa..48afcc1 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -1084,7 +1084,7 @@ int git_patch_parse(
patch->base.diff_opts.new_prefix = patch->new_prefix;
patch->base.diff_opts.flags |= GIT_DIFF_SHOW_BINARY;
- GIT_REFCOUNT_INC(patch);
+ GIT_REFCOUNT_INC(&patch->base);
*out = &patch->base;
done:
diff --git a/src/repository.c b/src/repository.c
index fe06963..90b778e 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -10,7 +10,6 @@
#include <ctype.h>
#include "git2/object.h"
-#include "git2/refdb.h"
#include "git2/sys/repository.h"
#include "common.h"
@@ -25,6 +24,7 @@
#include "refs.h"
#include "filter.h"
#include "odb.h"
+#include "refdb.h"
#include "remote.h"
#include "merge.h"
#include "diff_driver.h"
diff --git a/src/util.h b/src/util.h
index b85e03b..7c9a54f 100644
--- a/src/util.h
+++ b/src/util.h
@@ -297,22 +297,22 @@ typedef struct {
typedef void (*git_refcount_freeptr)(void *r);
#define GIT_REFCOUNT_INC(r) { \
- git_atomic_inc(&((git_refcount *)(r))->refcount); \
+ git_atomic_inc(&(r)->rc.refcount); \
}
#define GIT_REFCOUNT_DEC(_r, do_free) { \
- git_refcount *r = (git_refcount *)(_r); \
+ git_refcount *r = &(_r)->rc; \
int val = git_atomic_dec(&r->refcount); \
if (val <= 0 && r->owner == NULL) { do_free(_r); } \
}
#define GIT_REFCOUNT_OWN(r, o) { \
- ((git_refcount *)(r))->owner = o; \
+ (r)->rc.owner = o; \
}
-#define GIT_REFCOUNT_OWNER(r) (((git_refcount *)(r))->owner)
+#define GIT_REFCOUNT_OWNER(r) ((r)->rc.owner)
-#define GIT_REFCOUNT_VAL(r) git_atomic_get(&((git_refcount *)(r))->refcount)
+#define GIT_REFCOUNT_VAL(r) git_atomic_get((r)->rc.refcount)
static signed char from_hex[] = {