mbox series

[0/5] address W=1 warnings at staging/media/atomisp

Message ID cover.1599141140.git.mchehab+huawei@kernel.org
Headers show
Series address W=1 warnings at staging/media/atomisp | expand

Message

Mauro Carvalho Chehab Sept. 8, 2020, 10:12 a.m. UTC
The linux-media policy is to have zero warnings with W=1. However, when
I started using a Jenkins instance at https://builder.linuxtv.org to automate
such tests, I didn't notice that a bug at the scripts were just ignoring
warnings.

Now that this is fixed, we need to get rid of the warnings that got
re-introduced when the atomisp driver was reverted, as otherwise,
every time a common header is touched, we'll see messages like
those:

  Error/warnings:

  patches/0001-media-mxl5xx-remove-unused-including-linux-version.h.patch:

    allyesconfig: return code #0:
	../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:164:2: warning: function ‘atomisp_css2_dbg_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
	../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:170:2: warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
	../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:170:2: warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
	../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:176:2: warning: function ‘atomisp_css2_err_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
	../drivers/staging/media/atomisp/pci/atomisp_cmd.c:2814:31: warning: variable ‘stream_config’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/atomisp_cmd.c:2893:31: warning: variable ‘stream_config’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:1432:15: warning: variable ‘a0_max_id’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css_mipi.c:410:27: warning: variable ‘mipi_intermediate_info’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:1366:24: warning: variable ‘stream’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:2702:22: warning: variable ‘capture_pipe’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:2831:7: warning: variable ‘continuous’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:3534:15: warning: variable ‘num_output_pins’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:3937:55: warning: variable ‘vf_pp_binary’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:3937:38: warning: variable ‘preview_binary’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:3937:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:3936:26: warning: variable ‘me’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:5749:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:6013:7: warning: variable ‘continuous’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:6012:7: warning: variable ‘memory’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:7329:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:7459:26: warning: variable ‘num_vf_pp_stage’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css.c:10420:22: warning: variable ‘pipe_id’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css_param_shading.c:239:4: warning: variable ‘padded_width’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/sh_css_params.c:1099:36: warning: variable ‘row_padding’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c:237:31: warning: variable ‘isp_data_ptr’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:28: warning: variable ‘ver_num_isp’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:15: warning: variable ‘hor_num_isp’ set but not used [-Wunused-but-set-variable]
	../drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c:127:6: warning: variable ‘err’ set but not used [-Wunused-but-set-variable]
	In file included from ../drivers/staging/media/atomisp//pci/hive_isp_css_include/input_formatter.h:34,
	                 from ../drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c:55:
	../drivers/staging/media/atomisp//pci/hive_isp_css_common/host/input_formatter_local.h:118:27: warning: ‘input_formatter_alignment’ defined but not used [-Wunused-const-variable=]
	../drivers/staging/media/atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c:244:7: warning: variable ‘succes’ set but not used [-Wunused-but-set-variable]

    allyesconfig: return code #0:
	../drivers/media/v4l2-core/v4l2-ioctl.c: ../drivers/media/v4l2-core/v4l2-ioctl.c:3204 video_put_user() warn: check that 'ev32' doesn't leak information (struct has a hole after 'type')
	../drivers/media/v4l2-core/v4l2-ioctl.c: ../drivers/media/v4l2-core/v4l2-ioctl.c:3229 video_put_user() warn: check that 'vb32' doesn't leak information (struct has a hole after 'memory')
	../drivers/media/usb/em28xx/em28xx-video.c: ../drivers/media/usb/em28xx/em28xx-video.c:2841 em28xx_v4l2_init() parse error: turning off implications after 60 seconds
	../drivers/media/test-drivers/vivid/vivid-core.c: ../drivers/media/test-drivers/vivid/vivid-core.c:1200 vivid_create_instance() parse error: turning off implications after 60 seconds
	../drivers/media/test-drivers/vivid/vivid-core.c: ../drivers/media/test-drivers/vivid/vivid-core.c:1531 vivid_create_instance() parse error: __split_smt: function too hairy.  Giving up after 301 seconds

This series solve all atomisp W=1 warnings.

Mauro Carvalho Chehab (5):
  media: atomisp: get rid of some cleanup leftovers
  media: atomisp: print a warning if error while setting downscaler
  media: atomisp: get rid of unused vars
  media: atomisp: move a static constant out of a header file
  media: atomisp: get rid of -Wsuggest-attribute=format warnings

 .../staging/media/atomisp/pci/atomisp_cmd.c   |  6 ---
 .../media/atomisp/pci/atomisp_compat_css20.c  | 20 ++------
 .../staging/media/atomisp/pci/atomisp_v4l2.c  |  3 --
 .../host/input_formatter.c                    |  4 ++
 .../host/input_formatter_local.h              |  4 --
 .../staging/media/atomisp/pci/ia_css_env.h    |  4 +-
 .../isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c |  3 --
 .../kernels/sdis/sdis_2/ia_css_sdis2.host.c   |  3 --
 .../isp/kernels/vf/vf_1.0/ia_css_vf.host.c    |  5 ++
 .../atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c  |  4 +-
 drivers/staging/media/atomisp/pci/sh_css.c    | 47 ++-----------------
 .../staging/media/atomisp/pci/sh_css_mipi.c   | 12 -----
 .../media/atomisp/pci/sh_css_param_shading.c  | 25 +++-------
 .../staging/media/atomisp/pci/sh_css_params.c |  3 +-
 14 files changed, 29 insertions(+), 114 deletions(-)

Comments

Mauro Carvalho Chehab Sept. 3, 2020, 2:27 p.m. UTC | #1
Em Thu, 3 Sep 2020 17:07:24 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> escreveu:

> On Thu, Sep 03, 2020 at 03:57:32PM +0200, Mauro Carvalho Chehab wrote:
> > There are some warnings reported by gcc:
> > 	drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:164:2: warning: function ‘atomisp_css2_dbg_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > 	drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2: warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > 	drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2: warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > 	drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:176:2: warning: function ‘atomisp_css2_err_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > 
> > That are due to the usage of printf-like messages without
> > enabling the error checking logic.
> > 
> > Add the proper attributes in order to shut up such warnings.  
> 
> > +static int  __attribute__((format (printf, 1, 0)))
> > +atomisp_css2_dbg_ftrace_print(const char *fmt, va_list args)
> >  {
> >  	ftrace_vprintk(fmt, args);
> >  	return 0;
> >  }
> >    
> 
> Why not to drop it completely as well?

Because of this:

	make -s CC="gcc -fno-diagnostics-show-caret" CF=-D__CHECK_ENDIAN__ CONFIG_DEBUG_SECTION_MISMATCH=y  W=1 M=drivers/staging/media/atomisp/ 2>&1|grep -v ": In function ‘"
	drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:858:52: error: ‘ftrace_vprintk’ undeclared (first use in this function); did you mean ‘tracepoint’?
	drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:858:52: note: each undeclared identifier is reported only once for each function it appears in

Basically, kernel.h defines it as a macro:

	#define ftrace_vprintk(fmt, vargs)					\
	do {									\
		if (__builtin_constant_p(fmt)) {				\
			static const char *trace_printk_fmt __used		\
			  __attribute__((section("__trace_printk_fmt"))) =	\
				__builtin_constant_p(fmt) ? fmt : NULL;		\
										\
			__ftrace_vbprintk(_THIS_IP_, trace_printk_fmt, vargs);	\
		} else								\
			__ftrace_vprintk(_THIS_IP_, fmt, vargs);		\
	} while (0)

Now, a different thing would be to get rid of using trace as a debug
mechanism. Right now, I don't have any strong opinion, but I guess
that, while this driver is still at staging, it sounds good to be
able of using traces instead of dmesg for debugging purposes, but
to be honest, I didn't test yet to use ftrace for such purpose.

Thanks,
Mauro
Mauro Carvalho Chehab Sept. 8, 2020, 10:27 a.m. UTC | #2
Hi Greg,

Em Tue,  8 Sep 2020 12:12:21 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> The linux-media policy is to have zero warnings with W=1. However, when
> I started using a Jenkins instance at https://builder.linuxtv.org to automate
> such tests, I didn't notice that a bug at the scripts were just ignoring
> warnings.

Please ignore this patch series. It was sent by mistake instead
of a completely unrelated one.

This was already merged via the media tree.

Thanks,
Mauro