| Message ID | 20190625160103.GA13133@embeddedor (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | video: fbdev: s3c-fb: Mark expected switch fall-throughs | expand |
On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. [] > 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(+) diff --git a/Makefile b/Makefile index 5102b2bbd224..df909ffdfcdb 100644 --- a/Makefile +++ b/Makefile @@ -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)
On 6/25/19 11:52 AM, Joe Perches wrote: > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wimplicit-fallthrough, mark switch >> cases where we are expecting to fall through. > [] >> 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
On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote: > > In preparation to enabling -Wimplicit-fallthrough, mark switch > > cases where we are expecting to fall through. > [] > > 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.
On Tue, 2019-06-25 at 12:06 -0500, Gustavo A. R. Silva wrote: > > On 6/25/19 11:52 AM, Joe Perches wrote: > > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote: > > > In preparation to enabling -Wimplicit-fallthrough, mark switch > > > cases where we are expecting to fall through. > > [] > > > 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 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. > And I'll send a PR with a proper patch for the Makefile > during the next merge window. That's great. > 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. > 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.
On Tue, 2019-06-25 at 10:31 -0700, Kees Cook wrote: > On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote: > > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote: > > > In preparation to enabling -Wimplicit-fallthrough, mark switch > > > cases where we are expecting to fall through. > > [] > > > 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
On Tue, Jun 25, 2019 at 10:49:01AM -0700, Joe Perches wrote: > On Tue, 2019-06-25 at 10:31 -0700, Kees Cook wrote: > > On Tue, Jun 25, 2019 at 09:52:23AM -0700, Joe Perches wrote: > > > On Tue, 2019-06-25 at 11:01 -0500, Gustavo A. R. Silva wrote: > > > > In preparation to enabling -Wimplicit-fallthrough, mark switch > > > > cases where we are expecting to fall through. > > > [] > > > > 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? 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.
On 6/25/19 6:01 PM, Gustavo A. R. Silva wrote: > 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 <gustavo@embeddedor.com> Patch queued for v5.3, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/video/fbdev/s3c-fb.c b/drivers/video/fbdev/s3c-fb.c index 288300035164..f0b0643ab195 100644 --- a/drivers/video/fbdev/s3c-fb.c +++ b/drivers/video/fbdev/s3c-fb.c @@ -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) */
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 <gustavo@embeddedor.com> --- drivers/video/fbdev/s3c-fb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)