Re: [PATCH 01/11] misc: fallthrough fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/13/2017 03:41 PM, Karel Zak wrote:
On Tue, Jun 13, 2017 at 03:03:12PM +0200, Rüdiger Meier wrote:


On 06/13/2017 02:51 PM, Karel Zak wrote:
On Tue, Jun 13, 2017 at 01:16:57AM +0200, Rüdiger Meier wrote:
BTW just for cosmetics. Maybe we should rename UL_FALLTHROUGH to
__ul_fallthrough
like we have already __ul_calloc_size. IMO it would look more obvious to the
reader.

I don't have strong opinion about that, changed to one you proposed.

Thanks, let's see whether Karel likes this attribute at all ;)

Hmm...

   I don't like "meta-programming" where language keywords are
   replaced by local macros, for example we have fallback (in c.h) for all
   __attribute__ rather than define local macro for all attributes.

     __attribute__((fallthrough))

   should be fine on non-__GNUC__ as in this case __attribute__ is empty
   macro (see c.h).

   Yes, __ul_fallthrough seems better, and __attribute__((fallthrough))
   would be the best :-)

   It's strange if clang will use a different syntax for the same
   things. IMHO it would be better to ignore clang for this attribute.

Yes, Sami's last version just ignores clang. Seems that the different syntax
was for C++ only.

So branch 2017wk23 from git@xxxxxxxxxx:kerolasa/lelux-utiliteetit.git is the
last version for merging.

Cool, I see.

Now I read something about gcc and it seem it's able to recognize and
use comments, so  __attribute__((fallthrough)) is unnecessary.

The default -Wimplicit-fallthrough setting is '3' and it means "parse
comments"
So, do we really need this patch? ;-)

     https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/

and man gcc man page:

     -Wimplicit-fallthrough=0 disables the warning altogether.
     -Wimplicit-fallthrough=1 matches .* regular expression, any comment is used as fallthrough comment.
     -Wimplicit-fallthrough=2 case insensitively matches .*falls?[ \t-]*thr(ough|u).* regular expression.
     -Wimplicit-fallthrough=3 case sensitively matches one of the following regular expressions:
         -fallthrough
         @fallthrough@
         lint -fallthrough[ \t]*
         [ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?
         FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?
         [ \t.!]*(Else,? |Intentional(ly)? )?
         Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?
         [ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?
         fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?
     -Wimplicit-fallthrough=4 case sensitively matches one of the following regular expressions:
         -fallthrough
         @fallthrough@
         lint -fallthrough[ \t]*
         [ \t]*FALLTHR(OUGH|U)[ \t]*
     -Wimplicit-fallthrough=5 doesn’t recognize any comments as fallthrough comments, only attributes disable the warning.

sounds all we need is to add  -Wimplicit-fallthrough=3 to ./configur
No, -Wimplicit-fallthrough=3 is what we have already now by default.
Just tested. Without doing anything "/* fallthrough */" disables the
warning. But Sami fixed also other comments and places without comments.

So we could simply use "/* fallthrough */" everythere.

Note, -Wimplicit-fallthrough=2 would be even more relaxed, e.g.
  /* Fallthrough because bla. */  would work too.

cu,
Rudi
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux