Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 23 Jul 2019 02:31:24 +0000
From: Fangrui Song <>
To: Rich Felker <>
Subject: Re: [PATCH 1/5] fix warning dangling-else

On 2019-07-22, Rich Felker wrote:
>On Mon, Jul 22, 2019 at 02:07:36PM -0400, Issam Maghni wrote:
>> Signed-off-by: Issam Maghni <>
>> ---
>>  src/ctype/towctrans.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/src/ctype/towctrans.c b/src/ctype/towctrans.c
>> index 8f681018..bd0136dd 100644
>> --- a/src/ctype/towctrans.c
>> +++ b/src/ctype/towctrans.c
>> @@ -259,12 +259,14 @@ static wchar_t __towcase(wchar_t wc, int lower)
>>  	 || (unsigned)wc - 0xabc0 <= 0xfeff-0xabc0)
>>  		return wc;
>>  	/* special case because the diff between upper/lower is too big */
>> -	if (lower && (unsigned)wc - 0x10a0 < 0x2e)
>> +	if (lower && (unsigned)wc - 0x10a0 < 0x2e) {
>>  		if (wc>0x10c5 && wc != 0x10c7 && wc != 0x10cd) return wc;
>>  		else return wc + 0x2d00 - 0x10a0;
>> -	if (!lower && (unsigned)wc - 0x2d00 < 0x26)
>> +	}
>> +	if (!lower && (unsigned)wc - 0x2d00 < 0x26) {
>>  		if (wc>0x2d25 && wc != 0x2d27 && wc != 0x2d2d) return wc;
>>  		else return wc + 0x10a0 - 0x2d00;
>> +	}
>>  	if (lower && (unsigned)wc - 0x13a0 < 0x50)
>>  		return wc + 0xab70 - 0x13a0;
>>  	if (!lower && (unsigned)wc - 0xab70 < 0x50)
>> --
>> 2.22.0
>To clarify A. Wilcox's comment about "no chance of making it in", the
>coding style used in musl explicitly does not attempt to conform to
>the style rules that the warnings in this patch series are about. So
>there are questions of what the patches are attempting to address --
>is the goal to make clang stop spamming warnings, or to improve
>readability, or some mix of the two? If they were applied, would you
>be unhappy if the same warnings reappeared a few weeks layer due to
>new code somewhere else (in which case the request is really about a
>*policy* change, rather than an immediate code change)? Etc.
>I'm fairly neutral about the change above in patch 1, but opposed to
>most of the others. To me, visually, multiple levels of parentheses
>are hard to read. Much harder than understanding operator precedence.
>musl does make use of operator precedence, and assumes the reader is
>aware of it. In lots of places where precedence is relied upon,
>omission of spacing between some operators/operands is used as a hint
>to the reader of how the expression groups. In other places
>(especially &&/|| where it feels unnatural) it's usually not.
>Applying gratuitous style change commits is not without cost. Any bug
>fixes made after the style change commit will not apply to older
>versions of the software without manual fixups. Of course this happens
>for functional changes too, but in that case at least the change was
>well-motivated rather than being gratuitous. In the case of patch 1
>here, there's actually a pending replacement implementation for the
>whole file. I've held back on making the replacement because there
>were still some open questions about tuning it and it's considerably
>(a few kB) larger despite being much faster and more maintainable. So
>it probably doesn't make sense to apply a style change here now even
>if it were agreed to be desirable.
>What would really be much more welcome is a fix in configure for the
>default warnings options. Right now, if you use --enable-warnings, it
>enabled -Wall then disables known-unwanted ones by name; it's assumed
>that, without any -W options, the only warnings on will be ones for
>exceptionally egregious things that warrant the compiler enabling them
>by default. This was always true for GCC, but not for clang or
>Just always doing the -Wno-* part would help somewhat, but it won't
>keep up with new on-by-default warnings of clang, and if
>--enable-warnings is used, it won't keep up with new additions to
>-Wall that might not be wanted.
>For clang and cparser, adding -w to CFLAGS would let us start with a
>"clean slate" and add only the warnings we want. But for gcc, -w
>overrides any -W options, even subsequent ones, so we have to avoid
>passing -w if the compiler is real gcc.
>I've explored in the past getting rid of -Wall from --enable-warnings
>and instead explicitly adding each warning option that's definite or
>near-sure UB or hard constraint violations, rather than a style
>warning. This is probably the right course of action.

With the attached patch, gcc has just some warnings in src/ctype/towctrans.c

  supposedly it will be address soon: "In the case of patch 1 here, there's actually a pending replacement implementation for the whole file."

clang has a few more:

% grep -o '\[-.*\]' /tmp/clang.log | sort | uniq -c
 4 [-Wdangling-else]
10 [-Wignored-attributes]
     they are all in the form of `weak_alias(statfs, statfs64)`.
     these warnings will go away when the lfs64 things are fixed.
18 [-Wunknown-pragmas]
     src/math/fmal.c:167:15: warning: pragma STDC FENV_ACCESS ON is not supported, ignoring pragma [-Wunknown-pragmas]
             #pragma STDC FENV_ACCESS ON
     There is a long-standing bug
     "[llvm-dev] [cfe-dev] Why is #pragma STDC FENV_ACCESS not supported?" was a 2018 discussion on this topic.

[-Wdangling-else] and [-Wignored-attributes] will go away soon.

View attachment "musl.patch" of type "text/x-diff" (1345 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.