On Wed, Dec 14, 2016 at 1:13 AM, Florian Weimer wrote: > On 11/26/2016 07:14 AM, Jim Meyering wrote: > >> Here's a proposed ChangeLog entry: >> >> 2016-11-25 Jim Meyering >> >> Let gcc detect assert(a = 1) errors. >> * assert/assert.h (assert): Rewrite assert's definition so that a >> s/==/=/ typo, e.g., assert(errno = ENOENT) is not hidden from >> gcc's -Wparentheses by assert-added parentheses. The new >> definition uses "if (expr) /* empty */; else __assert_fail...", >> so gcc -Wall will now detect that type of error in an assert, too. >> The __STRICT_ANSI__ disjunct is to avoid the warning that >> -Wpedantic >> would otherwise issue for the use of ({...}). > > Isn't this warning suppressed for system headers with -Wpedantic? Thanks for the review. Good point. It's really for -ansi -pedantic, I suppose. Updated the log message. >> I would have preferred >> to use __extension__ to mark that, but doing so would mistakenly >> suppress warnings about any extension in the user-supplied "expr". >> E.g., "assert ( ({1;}) )" must continue to evoke a warning. >> https://bugzilla.redhat.com/1105335 > > The following is a bit silly. glibc adheres to the GNU standards, which > require that the ChangeLog only describes the change as such, not its > rationale. So the entry should probably look like this. > > Let GCC detect assert (a = 1) errors. > * assert/assert.h (assert): Introduce __STRICT_ANSI__ variant. > Use a GCC extension and an if statement for GNU compilers. Thanks. I didn't realize glibc used such a strict interpretation. Happy to adjust. How's this (also in the attached, rebased commit): Let gcc detect assert(a = 1) errors. * assert/assert.h (assert) Rewrite, retaining the old definintion when required, but otherwise putting the expression as-is in an "if" expression (hence, with no added parentheses) within a statement expression.