One problem with the mutt_array_size() macro added in 1196c859942e is that it's possible to accidentally use it on a pointer, rather than an array. This patch adds a (gcc-only) compile time check for that, using a technique from the Linux kernel source.
This patch, and the addition of the mutt_array_size() macro, is not without controversy. I would encourage those with interest to read the discussion in ticket 3899, where Derek voices several reasons not to use this kind of macro. In the end, I feel mutt_array_size() does, in some circumstances, add clarity to the code that is worth the trade-off. However, I am still willing to hear other's opinions on this. I'll hold off on committing this until next weekend, and am even willing to revert 1196c859942e if other mutt developers voice an opinion against. -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
# HG changeset patch # User Kevin McCarthy <ke...@8t8.us> # Date 1480811716 28800 # Sat Dec 03 16:35:16 2016 -0800 # Node ID 94b241da6cc7bc31dec48bda87a3f6252a9f8ad7 # Parent d72caeecf4af162b78fcabee2abfd0d7d62e6b07 Add gcc compile-time error check for mutt_array_size(). This technique is a combination of BUILD_BUG_ON() from the Linux kernel, and Vincent Lefèvre's array address comparison suggested in ticket 3899. Thanks also to Damien Riegel for pointing out the Linux kernel technique! diff --git a/lib.h b/lib.h --- a/lib.h +++ b/lib.h @@ -79,19 +79,42 @@ # define ISSPACE(c) isspace((unsigned char)c) # define strfcpy(A,B,C) strncpy(A,B,C), *(A+(C)-1)=0 # undef MAX # undef MIN # define MAX(a,b) ((a) < (b) ? (b) : (a)) # define MIN(a,b) ((a) < (b) ? (a) : (b)) +/* The compile-time error technique is from the Linux kernel source. + * Thanks to the kernel hackers for such a clever idea! */ +#ifdef __GNUC__ + +/* If passed true, this defines an illegal negative width bit field, + * which generates a compilation error. If passed false, it returns + * the size of a zero width bit field: 0. + * + * Note: a non-integer expression for the bit field size is not + * technically valid ISO C. Thus we enable it for GCC only, and label + * it an __extension__ to silence pedantic warnings. + */ +# define mutt_abort_compile_else_zero(e) __extension__ (sizeof(struct { int:-!!(e); })) + +/* This makes sure an actual array, not a pointer, is passed to + * mutt_array_size. If it is an array, 0 will be returned, otherwise + * compilation will be aborted. */ +# define mutt_must_be_array(a) mutt_abort_compile_else_zero((void *) &(a) != (void *) &(a)[0]) +#else +# define mutt_must_be_array(a) 0 +#endif /* __GNUC__ */ + /* Use this with care. If the compiler can't see the array - * definition, it obviously won't produce a correct result. */ -#define mutt_array_size(x) (sizeof (x) / sizeof ((x)[0])) + * definition, it obviously won't produce a correct result. + * With gcc, mutt_must_be_array() will check this. */ +#define mutt_array_size(x) (sizeof (x) / sizeof ((x)[0]) + mutt_must_be_array(x)) /* For mutt_format_string() justifications */ /* Making left 0 and center -1 is of course completely nonsensical, but * it retains compatibility for any patches that call mutt_format_string. * Once patches are updated to use FMT_*, these can be made sane. */ #define FMT_LEFT 0 #define FMT_RIGHT 1 #define FMT_CENTER -1
signature.asc
Description: PGP signature