This is the mail archive of the libc-alpha@sourceware.org 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] assert.h: allow gcc to detect assert(a = 1) errors


On Wed, Dec 14, 2016 at 1:13 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/26/2016 07:14 AM, Jim Meyering wrote:
>
>> Here's a proposed ChangeLog entry:
>>
>> 2016-11-25  Jim Meyering  <meyering@fb.com>
>>
>>         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.

Attachment: glibc-assert--do-not-suppress-warning-about-accidental-assignment.diff
Description: Text document


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