This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 06/13] Installed header hygiene (BZ#20366): Macros used in #if without checking whether they are defined.

On 08/29/2016 09:16 PM, Zack Weinberg wrote:
> sysdeps/unix/sysv/linux/bits/socket.h wants to know whether __flexarr
> will produce a real flexible array member -- specifically, one that
> doesn't alter sizeof(the structure containing it).  Rather than make
> its approximation to that condition any more complicated, I added a
> new macro to sys/cdefs.h, __flexarr_is_fake, which reveals exactly
> what it wants to know.  I also took the opportunity to flatten the
> rather messy conditional nest defining __flexarr.
> It seems to me that _LIBC should not appear in installed headers, but
> avoiding that for argp specifically would require more surgery than
> feels appropriate for this patch set.  It's possible that
> "#ifdef _LIBC" would be sufficient, but I wanted to be conservative.
> 	* argp/argp.h: Check whether _LIBC is defined before expanding it.
>         * posix/glob.h: Check whether __USE_XOPEN2K8 is defined instead
>         of expanding it.
>         * misc/sys/cdefs.h: Tidy up conditional nest defining __flexarr.
>         Define __flexarr_is_fake when the compiler does not support
>         flexible array members.
>         * sysdeps/unix/sysv/linux/bits/socket.h: Use __flexarr_is_fake
>         in definitions of struct cmsghdr and CMSG_DATA.

This patch fails to follow the Macro API best practice that all macros
be defined and that we should check for their values not their defined-ness
which can introduce subtle errors e.g. __flexarr_is_fake.

Please see:

At a high level I would expect _LIBC to always be defined as either 0 or 1.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]