video: fbdev: s3c-fb: Mark expected switch fall-throughs

PATCH In review Gustavo A. R. Silva 4 participants 8 messages Jun 25, 2019 – Jul 5, 2019

Subsystems (3)

Kbuild Framebuffer layer Samsung framebuffer driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2019-06-25 16:01 UTC #0 lore raw

PATCH 1/1 video: fbdev: s3c-fb: Mark expected switch fall-throughs

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_blank’:
drivers/video/fbdev/s3c-fb.c:811:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
sfb->enabled &= ~(1 << index);
~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
drivers/video/fbdev/s3c-fb.c:814:2: note: here
case FB_BLANK_NORMAL:
^~~~
LD [M] drivers/staging/greybus/gb-light.o
CC [M] drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gp10b.o
drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_check_var’:
drivers/video/fbdev/s3c-fb.c:286:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
var->transp.length = 1;
~~~~~~~~~~~~~~~~~~~^~~
drivers/video/fbdev/s3c-fb.c:288:2: note: here
case 18:
^~~~
drivers/video/fbdev/s3c-fb.c:314:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
var->transp.offset = 24;
~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/s3c-fb.c:316:2: note: here
case 24:
^~~~

Warning level 3 was used: -Wimplicit-fallthrough=3

Notice that, in this particular case, the code comments are modified in accordance with what GCC is expecting to find.

This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/video/fbdev/s3c-fb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/video/fbdev/s3c-fb.c +3 -3

index 288300035164..f0b0643ab195 100644
@@ -284,7 +284,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
 		/* 666 with one bit alpha/transparency */
 		var->transp.offset	= 18;
 		var->transp.length	= 1;
-		/* drop through */
+		/* fall through */
 	case 18:
 		var->bits_per_pixel	= 32;

@@ -312,7 +312,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
 	case 25:
 		var->transp.length	= var->bits_per_pixel - 24;
 		var->transp.offset	= 24;
-		/* drop through */
+		/* fall through */
 	case 24:
 		/* our 24bpp is unpacked, so 32bpp */
 		var->bits_per_pixel	= 32;

@@ -809,7 +809,7 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)
 	case FB_BLANK_POWERDOWN:
 		wincon &= ~WINCONx_ENWIN;
 		sfb->enabled &= ~(1 << index);
-		/* fall through to FB_BLANK_NORMAL */
+		/* fall through - to FB_BLANK_NORMAL */
 
 	case FB_BLANK_NORMAL:
 		/* disable the DMA and display 0x0 (black) */

Signature · 1 line

2.21.0

Joe Perches

2019-06-25 16:52 UTC #1 lore raw

Quoted 2 lines · Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.

[]

Quoted 2 lines

This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.

Just enable the thing already.

If you stopped trying to do it all yourself, others will help resolve any new build warnings.

For instance: a build of -next x86/64 defconfig has 2. nbd.

---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

Makefile +1 -0

index 5102b2bbd224..df909ffdfcdb 100644
@@ -690,6 +690,7 @@ endif # may-sync-config
 endif # $(dot-config)
 
 KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
+KBUILD_CFLAGS	+= $(call cc-option, -Wimplicit-fallthrough)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-truncation)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-overflow)

Gustavo A. R. Silva

author

2019-06-25 17:06 UTC #2 lore raw

Quoted 10 lines · Joe Perches

Quoted 2 lines · Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
[]
Quoted 2 lines
This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.
Just enable the thing already.
If you stopped trying to do it all yourself, others

What are you talking about?

Anyone can enable it, I'm adding this to every commit:

Warning level 3 was used: -Wimplicit-fallthrough=3

And I'll send a PR with a proper patch for the Makefile during the next merge window.

If had the power I would have enabled this option since day 1, so every developer can take care of their own code.

Lately, you are not being of much help, Joe.

--
Gustavo

Joe Perches

2019-06-25 17:37 UTC #3 lore raw

Quoted 11 lines · Gustavo A. R. Silva

Quoted 9 lines · Joe Perches
Quoted 2 lines · Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
[]
Quoted 2 lines
This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.
Just enable the thing already.

Quoted 7 lines

Quoted 1 line
If you stopped trying to do it all yourself, others
What are you talking about?
Anyone can enable it, I'm adding this to every commit:
Warning level 3 was used: -Wimplicit-fallthrough=3

No one does that by default and almost no one is helping eliminate these. Not even on th

Almost no one uses make W=<levels> either as it's generally extremely noisy and can emit a lot of false positives.

Quoted 2 lines

And I'll send a PR with a proper patch for the Makefile during the next merge window.

That's great.

Quoted 2 lines

If had the power I would have enabled this option since day 1, so every developer can take care of their own code.

You have always had the power to send a patch.

You also seem to believe the build needs to be completely clean before enabling the switch.

I don't.

Quoted 1 line

Lately, you are not being of much help, Joe.

<smile> What kind of help are you expecting?

I'm not submitting patches adding fallthough comments as I think that's not a good form. I've said so repeatedly.

I believe I suggested months ago you default enable the compiler switch.

So it's up to you to either do it or not.

Kees Cook

2019-06-25 17:31 UTC #4 lore raw

In reply to Joe Perches

Quoted 8 lines · Joe Perches

Quoted 2 lines · Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
[]
Quoted 2 lines
This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.
Just enable the thing already.

Linus has been pretty clear about not wanting warning options enabled without first fixing all the cases it warns about first. If that's changed, then sure, let's do it; but we went through a lot of coordination with sfr for linux-next nag-mail coverage (where maintainers are now fixing it themselves) when this came up during the last kernel summit, and this approach was the agreed solution.

Signature · 1 line

Kees Cook

Joe Perches

2019-06-25 17:49 UTC #5 lore raw

Quoted 4 lines · Kees Cook

Quoted 3 lines · Joe Perches
Quoted 2 lines · Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.

Quoted 8 lines

Quoted 5 lines
[]
Quoted 2 lines
This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.
Just enable the thing already.
Linus has been pretty clear about not wanting warning options enabled without first fixing all the cases it warns about first.

Hey Kees.

I don't recall that particular tidbit. Got a link?

cheers, Joe

Kees Cook

2019-06-25 20:18 UTC #6 lore raw

Quoted 9 lines · Joe Perches

Quoted 8 lines · Kees Cook
Quoted 7 lines · Joe Perches
Quoted 2 lines · Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
[]
Quoted 2 lines
This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Just enable the thing already.
Linus has been pretty clear about not wanting warning options enabled without first fixing all the cases it warns about first.
Hey Kees.
I don't recall that particular tidbit. Got a link?

It was spread out over the discussion around removing __deprecated, about enabling -Wvla, and in person at the kernel summit when asking what approach to take for -Wimplicit-fallthrough.

Signature · 1 line

Kees Cook

Bartlomiej Zolnierkiewicz

2019-07-05 15:16 UTC #7 lore raw

In reply to Gustavo A. R. Silva

Quoted 30 lines · Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
This patch fixes the following warnings:
drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_blank’:
drivers/video/fbdev/s3c-fb.c:811:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
   sfb->enabled &= ~(1 << index);
   ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
drivers/video/fbdev/s3c-fb.c:814:2: note: here
case FB_BLANK_NORMAL:
^~~~
LD [M] drivers/staging/greybus/gb-light.o
CC [M] drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gp10b.o
drivers/video/fbdev/s3c-fb.c: In function ‘s3c_fb_check_var’:
drivers/video/fbdev/s3c-fb.c:286:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
   var->transp.length = 1;
   ~~~~~~~~~~~~~~~~~~~^~~
drivers/video/fbdev/s3c-fb.c:288:2: note: here
case 18:
^~~~
drivers/video/fbdev/s3c-fb.c:314:22: warning: this statement may fall through [-Wimplicit-fallthrough=]
   var->transp.offset = 24;
   ~~~~~~~~~~~~~~~~~~~^~~~
drivers/video/fbdev/s3c-fb.c:316:2: note: here
case 24:
^~~~
Warning level 3 was used: -Wimplicit-fallthrough=3

Quoted 7 lines

Notice that, in this particular case, the code comments are modified in accordance with what GCC is expecting to find.
This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough.
Signed-off-by Gustavo A. R. Silva <[email protected]>

Patch queued for v5.3, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics