compat/obstack: fix -Wcast-function-type warnings

GCC 8 introduced the new -Wcast-function-type warning, which is implied by -Wextra (which, in turn is enabled in our DEVELOPER flags). When building Git with GCC 8 and this warning enabled on a non-glibc platform [1], one is greeted with a screenful of compiler warnings/errors: compat/obstack.c: In function '_obstack_begin': compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type] h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun; ^ compat/obstack.c:163:16: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(void *, struct _obstack_chunk *)' [-Werror=cast-function-type] h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun; ^ compat/obstack.c:116:8: error: cast between incompatible function types from 'struct _obstack_chunk * (*)(void *, long int)' to 'struct _obstack_chunk * (*)(long int)' [-Werror=cast-function-type] : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size))) ^ compat/obstack.c:168:22: note: in expansion of macro 'CALL_CHUNKFUN' chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size); ^~~~~~~~~~~~~ <snip> 'struct obstack' stores pointers to two functions to allocate and free "chunks", and depending on how obstack is used, these functions take either one parameter (like standard malloc() and free() do; this is how we use it in 'kwset.c') or two parameters. Presumably to reduce memory footprint, a single field is used to store the function pointer for both signatures, and then it's casted to the appropriate signature when the function pointer is accessed. These casts between function pointers with different number of parameters are what trigger those compiler errors. Modify 'struct obstack' to use unions to store function pointers with different signatures, and then use the union member with the appropriate signature when accessing these function pointers. This eliminates the need for those casts, and thus avoids this compiler error. [1] Compiling 'compat/obstack.c' on a platform with glibc is sort of a noop, see the comment before '# define ELIDE_CODE', so this is not an issue on common Linux distros. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Jan 17, 2019 at 02:29 UTC 764473d25772aeb6e1f02743bc13a6da8eb18cc3
2 files changed +20 -15
compat/obstack.c
+9 -8
@@ -112,15 +112,15 @@ compat_symbol (libc, _obstack_compat, _obstack, GLIBC_2_0);
112
113 # define CALL_CHUNKFUN(h, size) \
114 (((h) -> use_extra_arg) \
115 - ? (*(h)->chunkfun) ((h)->extra_arg, (size)) \
116 - : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
115 + ? (*(h)->chunkfun.extra) ((h)->extra_arg, (size)) \
116 + : (*(h)->chunkfun.plain) ((size)))
117
118 # define CALL_FREEFUN(h, old_chunk) \
119 do { \
120 if ((h) -> use_extra_arg) \
121 - (*(h)->freefun) ((h)->extra_arg, (old_chunk)); \
121 + (*(h)->freefun.extra) ((h)->extra_arg, (old_chunk)); \
122 else \
123 - (*(void (*) (void *)) (h)->freefun) ((old_chunk)); \
123 + (*(h)->freefun.plain) ((old_chunk)); \
124 } while (0)
125
126
@@ -159,8 +159,8 @@ _obstack_begin (struct obstack *h,
159 size = 4096 - extra;
160 }
161
162 - h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
163 - h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
162 + h->chunkfun.plain = chunkfun;
163 + h->freefun.plain = freefun;
164 h->chunk_size = size;
165 h->alignment_mask = alignment - 1;
166 h->use_extra_arg = 0;
@@ -206,8 +206,9 @@ _obstack_begin_1 (struct obstack *h, int size, int alignment,
206 size = 4096 - extra;
207 }
208
209 - h->chunkfun = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
210 - h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
209 + h->chunkfun.extra = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
210 + h->freefun.extra = (void (*) (void *, struct _obstack_chunk *)) freefun;
211 +
212 h->chunk_size = size;
213 h->alignment_mask = alignment - 1;
214 h->extra_arg = arg;
compat/obstack.h
+11 -7
@@ -160,11 +160,15 @@ struct obstack /* control current object in current chunk */
160 void *tempptr;
161 } temp; /* Temporary for some macros. */
162 int alignment_mask; /* Mask of alignment for each object. */
163 - /* These prototypes vary based on `use_extra_arg', and we use
164 - casts to the prototypeless function type in all assignments,
165 - but having prototypes here quiets -Wstrict-prototypes. */
166 - struct _obstack_chunk *(*chunkfun) (void *, long);
167 - void (*freefun) (void *, struct _obstack_chunk *);
163 + /* These prototypes vary based on `use_extra_arg'. */
164 + union {
165 + void *(*plain) (long);
166 + struct _obstack_chunk *(*extra) (void *, long);
167 + } chunkfun;
168 + union {
169 + void (*plain) (void *);
170 + void (*extra) (void *, struct _obstack_chunk *);
171 + } freefun;
172 void *extra_arg; /* first arg for chunk alloc/dealloc funcs */
173 unsigned use_extra_arg:1; /* chunk alloc/dealloc funcs take extra arg */
174 unsigned maybe_empty_object:1;/* There is a possibility that the current
@@ -235,10 +239,10 @@ extern void (*obstack_alloc_failed_handler) (void);
239 (void (*) (void *, void *)) (freefun), (arg))
240
241 #define obstack_chunkfun(h, newchunkfun) \
238 - ((h) -> chunkfun = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
242 + ((h)->chunkfun.extra = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
243
244 #define obstack_freefun(h, newfreefun) \
241 - ((h) -> freefun = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
245 + ((h)->freefun.extra = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
246
247 #define obstack_1grow_fast(h,achar) (*((h)->next_free)++ = (achar))
248