Page MenuHomeFreeBSD

Remove potential identifier conflict in the EV_SET macro.
ClosedPublic

Authored by dab on Jun 6 2018, 7:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 1, 6:21 PM
Unknown Object (File)
Mon, Nov 25, 1:59 PM
Unknown Object (File)
Oct 24 2024, 9:34 PM
Unknown Object (File)
Oct 24 2024, 9:34 PM
Unknown Object (File)
Oct 24 2024, 9:34 PM
Unknown Object (File)
Oct 24 2024, 9:34 PM
Unknown Object (File)
Oct 24 2024, 9:18 PM
Unknown Object (File)
Oct 1 2024, 11:48 PM

Details

Summary

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.

I should note that the goal of making this change is primarily to remove the last (potential) open issue in PR43905 and close it. One could certainly make the argument that the existing solution is sufficient and the bug should just be closed as-is. I'm open to hearing such an argument.

PR: 43905

Test Plan

Run the kqueue system tests.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17059
Build 16922: arc lint + arc unit

Event Timeline

sys/sys/event.h
52

I could have done this as just a straight replacement for the old definition as from what I understand all platforms compile with at least C99 support. However, I decided to take this approach in case someone is still building with a pre-C99 compiler. I saw several other places in the code base that check for C99/pre-C99.

jilles requested changes to this revision.Jun 6 2018, 9:41 PM

Thanks for working on this, but I think pedantic compilation modes should be kept working.

sys/sys/event.h
52

Supporting pre-C99 here is not immediately unreasonable, since this file is included in user code.

Apart from that, standard C++ does not support compound literals, so using them may cause a warning. Both GCC and Clang support compound literals in C++ as an extension, but with different semantics if a pointer is taken to one (which does not happen here).

There should be a check defined(__STDC_VERSION__) so that compiling C++ code with -Wundef does not generate a warning.

This revision now requires changes to proceed.Jun 6 2018, 9:41 PM

Add check for STDC_VERSION being defined before attempting to check its value.

dab marked 2 inline comments as done.Jun 7 2018, 1:13 AM

Revert an unintended change to the original EV_SET macro that crept into my last revision.

@jilles, @jmg : Any further input on this change?

Looks good to me apart from the comment. Sorry for the delay.

sys/sys/event.h
67

Pre-C99 or C++

This revision is now accepted and ready to land.Jun 26 2018, 8:32 PM