Re: strengthening ARRAY_CARDINALITY

[Mike Frysinger]

On 19 Jan 2017 17:13, Jim Meyering wrote:
> On Thu, Jan 19, 2017 at 5:06 PM, Mike Frysinger wrote:
> > On 19 Jan 2017 22:20, Pádraig Brady wrote:
> >> On 19/01/17 21:52, Eric Blake wrote:
> >> > On the qemu list, it was pointed out that code that uses
> >> > ARRAY_CARDINALITY() might still compile even after it has been
> >> > refactored to use a pointer (probably conversion of an array into
> >> > dynamic allocation), but that you can add a compile-time check with
> >> > new-enough gcc/clang to catch this.
> >> >
> >> > I'm also wondering if we should promote ARRAY_CARDINALITY into a
> >> > full-fledged gnulib module (several gnulib files define it in .c files,
> >> > but leave projects to re-define their own; coreutils' is in system.h).
> >> >
> >> > The qemu list spells their macro ARRAY_SIZE, and
> >> > QEMU_BUILD_BUG_ON_ZERO(x) is equivalent to our verify_expr(!x, 0), but
> >> > I'm wondering if we should similarly strengthen coreutils' macro (with
> >> > appropriate guards for new-enough gcc, since we target more compilers
> >> > than qemu):
> >> >
> >> > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04118.html
> >> >
> >> > +/*
> >> > + * &(x)[0] is always a pointer - if it's same type as x then the
> >> > argument is a
> >> > + * pointer, not an array.
> >> > + */
> >> > +#define QEMU_IS_ARRAY(x) (!__builtin_types_compatible_p(typeof(x), \
> >> > +                                                        
> >> > typeof(&(x)[0])))
> >> >  #ifndef ARRAY_SIZE
> >> > -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> >> > +#define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])) + \
> >> > +                       QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(x)))
> >> >  #endif
> >>
> >> gnulib module + extra checks + name change to ARRAY_SIZE sound good!
> >> For a convenience feature, ARRAY_CARDINALITY is not a convenient name.
> >
> > yeah, "cardinality" sounds more like someone trying to show off their
> > familiarity with the english language ;).  then again, my spell checker
> > says it's not a word.
> >
> > linux, glibc, bootloaders, use ARRAY_SIZE.  types use "size" -- it's
> > size_t, not cardinality_t.  this is the first time i've seen this name
> > used myself.
> 
> FYI, I chose "cardinality" over "size" quite deliberately: the former
> refers to the number of things (typically in a set), and while "size"
> may also refer to that number, it is often used to refer to the number
> of bytes, which is usually a larger number.

a fair point.  i think in practice, people are used to the behavior
of the macro so it's not an issue.  plus, sizeof already does this.
-mike

signature.asc
Description: Digital signature