svn commit: r335765 - head/sys/sys

[Bryan Drewery]

On 6/28/2018 11:45 AM, Justin Hibbits wrote:
> Hi David,
> 
> On Thu, Jun 28, 2018 at 12:01 PM David Bright <dab at freebsd.org> wrote:
>>
>> Author: dab
>> Date: Thu Jun 28 17:01:04 2018
>> New Revision: 335765
>> URL: https://svnweb.freebsd.org/changeset/base/335765
>>
>> Log:
>>   Remove potential identifier conflict in the EV_SET macro.
>>
>>   PR43905 pointed out a problem with the EV_SET macro if the passed
>>   struct kevent pointer were specified with an expression with side
>>   effects (e.g., "kevp++"). This was fixed in rS110241, but by using a
>>   local block that defined an internal variable (named "kevp") to get
>>   the pointer value once. This worked, but could cause issues if an
>>   existing variable named "kevp" is in scope. To avoid that issue,
>>   jilles@ pointed out that "C99 compound literals and designated
>>   initializers allow doing this cleanly using a macro". This change
>>   incorporates that suggestion, essentially verbatim from jilles@
>>   comment on PR43905, except retaining the old definition for pre-C99 or
>>   non-STDC (e.g., C++) compilers.
>>
>>   PR:   43905
>>   Submitted by: Jilles Tjoelker (jilles@)
>>   Reported by:  Lamont Granquist <lamont at scriptkiddie.org>
>>   Reviewed by:  jmg (no comments), jilles
>>   MFC after:    1 week
>>   Sponsored by: Dell EMC
>>   Differential Revision:        https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=43905
>>
>> Modified:
>>   head/sys/sys/event.h
>>
>> Modified: head/sys/sys/event.h
>> ==============================================================================
>> --- head/sys/sys/event.h        Thu Jun 28 15:30:51 2018        (r335764)
>> +++ head/sys/sys/event.h        Thu Jun 28 17:01:04 2018        (r335765)
>> @@ -49,7 +49,26 @@
>>  #define EVFILT_EMPTY           (-13)   /* empty send socket buf */
>>  #define EVFILT_SYSCOUNT                13
>>
>> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
>>  #define EV_SET(kevp_, a, b, c, d, e, f) do {   \
>> +    *(kevp_) = (struct kevent){                        \
>> +       .ident = (a),                           \
>> +       .filter = (b),                          \
>> +       .flags = (c),                           \
>> +       .fflags = (d),                          \
>> +       .data = (e),                            \
>> +       .udata = (f),                           \
>> +       .ext[0] = 0,                            \
>> +       .ext[1] = 0,                            \
>> +       .ext[2] = 0,                            \
>> +       .ext[3] = 0,                            \
>> +    };                                         \
>> +} while(0)
>> +#else /* Pre-C99 or not STDC (e.g., C++) */
>> +/* The definition of the local variable kevp could possibly conflict
>> + * with a user-defined value passed in parameters a-f.
>> + */
>> +#define EV_SET(kevp_, a, b, c, d, e, f) do {   \
>>         struct kevent *kevp = (kevp_);          \
>>         (kevp)->ident = (a);                    \
>>         (kevp)->filter = (b);                   \
>> @@ -62,6 +81,7 @@
>>         (kevp)->ext[2] = 0;                     \
>>         (kevp)->ext[3] = 0;                     \
>>  } while(0)
>> +#endif
>>
>>  struct kevent {
>>         __uintptr_t     ident;          /* identifier for this event */
>>
> 
> This breaks gcc builds, with the following errors:
> 
> 18:02:13 /usr/src/bin/pwait/pwait.c: In function 'main':
> 18:02:13 /usr/src/bin/pwait/pwait.c:144: warning: initialized field overwritten
> 18:02:13 /usr/src/bin/pwait/pwait.c:144: warning: (near initialization
> for '(anonymous).ext')
> 18:02:13 /usr/src/bin/pwait/pwait.c:144: warning: initialized field overwritten
> 18:02:13 /usr/src/bin/pwait/pwait.c:144: warning: (near initialization
> for '(anonymous).ext')
> 18:02:13 /usr/src/bin/pwait/pwait.c:144: warning: initialized field overwritten
> 18:02:13 /usr/src/bin/pwait/pwait.c:144: warning: (near initialization
> for '(anonymous).ext')
> 18:02:13 /usr/src/bin/pwait/pwait.c:158: warning: initialized field overwritten
> 18:02:13 /usr/src/bin/pwait/pwait.c:158: warning: (near initialization
> for '(anonymous).ext')
> 18:02:13 /usr/src/bin/pwait/pwait.c:158: warning: initialized field overwritten
> 18:02:13 /usr/src/bin/pwait/pwait.c:158: warning: (near initialization
> for '(anonymous).ext')
> 18:02:13 /usr/src/bin/pwait/pwait.c:158: warning: initialized field overwritten
> 18:02:13 /usr/src/bin/pwait/pwait.c:158: warning: (near initialization
> for '(anonymous).ext')
> 
> This can be seen in the tinderbox (https://ci.freebsd.org/tinderbox)
> 
> I encountered this as a failure in building usr.sbin/camdd, which
> shows the same type of error.

I think in this case it may break ports and other external builds too.
If possible please try to fix the macro to avoid the issue. I didn't
analyze it to see if it is feasible though.


-- 
Regards,
Bryan Drewery

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20180628/f75f2bc5/attachment.sig>