Re: [patch 014/102] llist: introduce llist_entry_safe()

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

 



[ Coming back to this, because it was annoying me ]

On Tue, Oct 11, 2016 at 4:06 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> HOWEVER - and this is really annoying - we'd need to use "-std=gnu99/gnu11"

Hmm.

I just tried it again after a long time, and "-std=gnu99" seems to
work fine for me on the kernel these days. The old "compound literal"
issues we had at some point seem gone.

Maybe we've cleaned everything up that used to cause problems. Or
maybe I just mis-remembered and we should have done this long ago?

If we switched the kernel over from gnu89 to gnu99, we could use a
couple of modern C features that are really nice. The main one is
variable declarations in loops, which isn't just syntactic sugar: it
makes it much easier to reduce the scope of variables naturally, but
it also makes it easier to do legible macros for for-loops when you
can declare your helper variables inside the for-statement.

The one thing I noticed while doing this was that switching to
std=gnu99 results in some funky new warnings. In particular, drm has a
number of warnings like this:

  drivers/gpu/drm/i915/gt/intel_sseu.c: In function ‘intel_sseu_make_rpcs’:
  drivers/gpu/drm/i915/gt/intel_sseu.c:64:10: warning: left shift of
negative value [-Wshift-negative-value]
     64 |     ~(~0 << (hweight8(ctx_sseu.subslice_mask) / 2));
        |          ^~

and that seems to be because c99 allows ones-complement machines to do
odd things with left shifts.

It doesn't make any sensible difference on a two's-complement machine,
and it's insane that c99 changed the semantics to be worse for
something that will never matter, but there you have it.

Language standards people seemingly still not realizing that undefined
behavior is a *bad* thing, not a good thing, and adding it for no good
reason? Welcome to the mad-house.

Anyway, that warning is easy enough to shut up. Would people mind
testing this patch? Does it cause problems on other architectures?
Maybe the drm code could even be made to use unsigned values for their
shifts?

Because I'd love to finally be able to do things like that

  #define llist_for_each_entry(pos, node, member) \
        for (struct llist_node *_lptr = (node); \
        (pos) = llist_entry(_lptr, typeof(*(pos)), member), !!_lptr; \
        _lptr = (pos)->member.next)

and not have that odd "member_address_is_nonnull()" because of how we
convert back and forth.

It would also allow us to force the caller to pointlessly have to
declare a temporary variable for the loop for the safe versions.

I only tested gnu99 - which is sufficient for the above - and didn't
see if gnu11 ends up causing more issues.. And I didn't actually test
any of the code modifications that this would allow us to do.

Adding Arnd (because he seems to like compiler updates, and works with
compiler warnings ;) and the kbuild people and arch list.

             Linus
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index ffd7a912fc46..cb9ab8a70b8e 100644
--- a/Makefile
+++ b/Makefile
@@ -397,7 +397,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
 HOSTCC       = gcc
 HOSTCXX      = g++
 KBUILD_HOSTCFLAGS   := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
-		-fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \
+		-fomit-frame-pointer -std=gnu99 $(HOST_LFS_CFLAGS) \
 		$(HOSTCFLAGS)
 KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS)
 KBUILD_HOSTLDFLAGS  := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS)
@@ -459,7 +459,7 @@ KBUILD_CFLAGS   := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
 		   -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \
 		   -Werror=implicit-function-declaration -Werror=implicit-int \
 		   -Wno-format-security \
-		   -std=gnu89
+		   -std=gnu99
 KBUILD_CPPFLAGS := -D__KERNEL__
 KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=
@@ -699,6 +699,7 @@ KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-truncation)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-overflow)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, address-of-packed-member)
+KBUILD_CFLAGS	+= $(call cc-disable-warning, shift-negative-value)
 
 ifdef CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE
 KBUILD_CFLAGS += -O2

[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux