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

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

 



Hi Sami,

On 06/11/2017 10:36 AM, Sami Kerola wrote:
On 9 June 2017 at 22:10, Rüdiger Meier <sweet_f_a@xxxxxx> wrote:

Sorry this was broken. But I've checked now all clang and gcc versions.
Looks
like for clang the warning is only enabled when using C++ (clang++). So the
fallthrough warning appears currently only on gcc>=7 and this works to
disable
it in a compatible way:

#ifndef __has_cpp_attribute
# define __has_cpp_attribute(x) 0
#endif

#if __has_cpp_attribute(fallthrough)
# define UL_FALLTHROUGH __attribute__((fallthrough))
#else
# define UL_FALLTHROUGH /* fall through */
#endif

Thank you for review Rudi. I had a look of the c.h again and did the
change a little bit differently:

#ifdef __has_cpp_attribute
# if __has_cpp_attribute(deprecated)

Is this a typoo? s/deprecated/fallthrough

#  define UL_FALLTHROUGH __attribute__((fallthrough))
# elif __has_cpp_attribute(clang::fallthrough)
#  define UL_FALLTHROUGH [[clang::fallthrough]]

I still wonder. Have you really seen such a fallthrough warning from clang?
I've even tried with explicit -Wimplicit-fallthrough but could not get one.

As far as I understood these namespaced "clang::" attributes are for C++ only.

man clang:
   Non-standard C++11 Attributes
       Clang's non-standard C++11 attributes live in the clang attribute namespace

So IMO it makes no sense to use clang::fallthrough for C code.

# endif
#endif
#ifndef UL_FALLTHROUGH
# define UL_FALLTHROUGH /* FALLTHRU */
#endif

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.
Also I would use it without semicolon because some compilers may complain about
useless ";" statements.

cu,
Rudi

The above is now part of my remote branch '2017wk23'.

--
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