linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: davinci-isif: avoid uninitialized variable use
@ 2019-03-22 14:34 Arnd Bergmann
  2019-03-22 16:01 ` Nathan Chancellor
  2019-03-26 10:23 ` Lad, Prabhakar
  0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:34 UTC (permalink / raw)
  To: Lad, Prabhakar, Mauro Carvalho Chehab
  Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
	Arnd Bergmann, Philipp Zabel, Kieran Bingham, Wenwen Wang,
	linux-media, linux-kernel

clang warns about a possible variable use that gcc never
complained about:

drivers/media/platform/davinci/isif.c:982:32: error: variable 'frame_size' is uninitialized when used here
      [-Werror,-Wuninitialized]
                dm365_vpss_set_pg_frame_size(frame_size);
                                             ^~~~~~~~~~
drivers/media/platform/davinci/isif.c:887:2: note: variable 'frame_size' is declared here
        struct vpss_pg_frame_size frame_size;
        ^
1 error generated.

There is no initialization for this variable at all, and there
has never been one in the mainline kernel, so we really should
not put that stack data into an mmio register.

On the other hand, I suspect that gcc checks the condition
more closely and notices that the global
isif_cfg.bayer.config_params.test_pat_gen flag is initialized
to zero and never written to from any code path, so anything
depending on it can be eliminated.

To shut up the clang warning, just remove the dead code manually,
it has probably never been used because any attempt to do so
would have resulted in undefined behavior.

Fixes: 63e3ab142fa3 ("V4L/DVB: V4L - vpfe capture - source for ISIF driver on DM365")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/platform/davinci/isif.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c
index 47cecd10eb9f..2dee9af6d413 100644
--- a/drivers/media/platform/davinci/isif.c
+++ b/drivers/media/platform/davinci/isif.c
@@ -884,9 +884,7 @@ static int isif_set_hw_if_params(struct vpfe_hw_if_param *params)
 static int isif_config_ycbcr(void)
 {
 	struct isif_ycbcr_config *params = &isif_cfg.ycbcr;
-	struct vpss_pg_frame_size frame_size;
 	u32 modeset = 0, ccdcfg = 0;
-	struct vpss_sync_pol sync;
 
 	dev_dbg(isif_cfg.dev, "\nStarting isif_config_ycbcr...");
 
@@ -974,13 +972,6 @@ static int isif_config_ycbcr(void)
 		/* two fields are interleaved in memory */
 		regw(0x00000249, SDOFST);
 
-	/* Setup test pattern if enabled */
-	if (isif_cfg.bayer.config_params.test_pat_gen) {
-		sync.ccdpg_hdpol = params->hd_pol;
-		sync.ccdpg_vdpol = params->vd_pol;
-		dm365_vpss_set_sync_pol(sync);
-		dm365_vpss_set_pg_frame_size(frame_size);
-	}
 	return 0;
 }
 
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: davinci-isif: avoid uninitialized variable use
  2019-03-22 14:34 [PATCH] media: davinci-isif: avoid uninitialized variable use Arnd Bergmann
@ 2019-03-22 16:01 ` Nathan Chancellor
  2019-03-26 10:23 ` Lad, Prabhakar
  1 sibling, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2019-03-22 16:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Lad, Prabhakar, Mauro Carvalho Chehab, clang-built-linux,
	Nick Desaulniers, Philipp Zabel, Kieran Bingham, Wenwen Wang,
	linux-media, linux-kernel

On Fri, Mar 22, 2019 at 03:34:22PM +0100, Arnd Bergmann wrote:
> clang warns about a possible variable use that gcc never
> complained about:
> 
> drivers/media/platform/davinci/isif.c:982:32: error: variable 'frame_size' is uninitialized when used here
>       [-Werror,-Wuninitialized]
>                 dm365_vpss_set_pg_frame_size(frame_size);
>                                              ^~~~~~~~~~
> drivers/media/platform/davinci/isif.c:887:2: note: variable 'frame_size' is declared here
>         struct vpss_pg_frame_size frame_size;
>         ^
> 1 error generated.
> 
> There is no initialization for this variable at all, and there
> has never been one in the mainline kernel, so we really should
> not put that stack data into an mmio register.
> 
> On the other hand, I suspect that gcc checks the condition
> more closely and notices that the global
> isif_cfg.bayer.config_params.test_pat_gen flag is initialized
> to zero and never written to from any code path, so anything
> depending on it can be eliminated.
> 
> To shut up the clang warning, just remove the dead code manually,
> it has probably never been used because any attempt to do so
> would have resulted in undefined behavior.
> 
> Fixes: 63e3ab142fa3 ("V4L/DVB: V4L - vpfe capture - source for ISIF driver on DM365")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  drivers/media/platform/davinci/isif.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c
> index 47cecd10eb9f..2dee9af6d413 100644
> --- a/drivers/media/platform/davinci/isif.c
> +++ b/drivers/media/platform/davinci/isif.c
> @@ -884,9 +884,7 @@ static int isif_set_hw_if_params(struct vpfe_hw_if_param *params)
>  static int isif_config_ycbcr(void)
>  {
>  	struct isif_ycbcr_config *params = &isif_cfg.ycbcr;
> -	struct vpss_pg_frame_size frame_size;
>  	u32 modeset = 0, ccdcfg = 0;
> -	struct vpss_sync_pol sync;
>  
>  	dev_dbg(isif_cfg.dev, "\nStarting isif_config_ycbcr...");
>  
> @@ -974,13 +972,6 @@ static int isif_config_ycbcr(void)
>  		/* two fields are interleaved in memory */
>  		regw(0x00000249, SDOFST);
>  
> -	/* Setup test pattern if enabled */
> -	if (isif_cfg.bayer.config_params.test_pat_gen) {
> -		sync.ccdpg_hdpol = params->hd_pol;
> -		sync.ccdpg_vdpol = params->vd_pol;
> -		dm365_vpss_set_sync_pol(sync);
> -		dm365_vpss_set_pg_frame_size(frame_size);
> -	}
>  	return 0;
>  }
>  
> -- 
> 2.20.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: davinci-isif: avoid uninitialized variable use
  2019-03-22 14:34 [PATCH] media: davinci-isif: avoid uninitialized variable use Arnd Bergmann
  2019-03-22 16:01 ` Nathan Chancellor
@ 2019-03-26 10:23 ` Lad, Prabhakar
  1 sibling, 0 replies; 3+ messages in thread
From: Lad, Prabhakar @ 2019-03-26 10:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mauro Carvalho Chehab, clang-built-linux, Nick Desaulniers,
	Nathan Chancellor, Philipp Zabel, Kieran Bingham, Wenwen Wang,
	linux-media, LKML

On Fri, Mar 22, 2019 at 2:34 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> clang warns about a possible variable use that gcc never
> complained about:
>
> drivers/media/platform/davinci/isif.c:982:32: error: variable 'frame_size' is uninitialized when used here
>       [-Werror,-Wuninitialized]
>                 dm365_vpss_set_pg_frame_size(frame_size);
>                                              ^~~~~~~~~~
> drivers/media/platform/davinci/isif.c:887:2: note: variable 'frame_size' is declared here
>         struct vpss_pg_frame_size frame_size;
>         ^
> 1 error generated.
>
> There is no initialization for this variable at all, and there
> has never been one in the mainline kernel, so we really should
> not put that stack data into an mmio register.
>
> On the other hand, I suspect that gcc checks the condition
> more closely and notices that the global
> isif_cfg.bayer.config_params.test_pat_gen flag is initialized
> to zero and never written to from any code path, so anything
> depending on it can be eliminated.
>
> To shut up the clang warning, just remove the dead code manually,
> it has probably never been used because any attempt to do so
> would have resulted in undefined behavior.
>
> Fixes: 63e3ab142fa3 ("V4L/DVB: V4L - vpfe capture - source for ISIF driver on DM365")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/platform/davinci/isif.c | 9 ---------
>  1 file changed, 9 deletions(-)
>

Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Cheers,
--Prabhakar Lad

> diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c
> index 47cecd10eb9f..2dee9af6d413 100644
> --- a/drivers/media/platform/davinci/isif.c
> +++ b/drivers/media/platform/davinci/isif.c
> @@ -884,9 +884,7 @@ static int isif_set_hw_if_params(struct vpfe_hw_if_param *params)
>  static int isif_config_ycbcr(void)
>  {
>         struct isif_ycbcr_config *params = &isif_cfg.ycbcr;
> -       struct vpss_pg_frame_size frame_size;
>         u32 modeset = 0, ccdcfg = 0;
> -       struct vpss_sync_pol sync;
>
>         dev_dbg(isif_cfg.dev, "\nStarting isif_config_ycbcr...");
>
> @@ -974,13 +972,6 @@ static int isif_config_ycbcr(void)
>                 /* two fields are interleaved in memory */
>                 regw(0x00000249, SDOFST);
>
> -       /* Setup test pattern if enabled */
> -       if (isif_cfg.bayer.config_params.test_pat_gen) {
> -               sync.ccdpg_hdpol = params->hd_pol;
> -               sync.ccdpg_vdpol = params->vd_pol;
> -               dm365_vpss_set_sync_pol(sync);
> -               dm365_vpss_set_pg_frame_size(frame_size);
> -       }
>         return 0;
>  }
>
> --
> 2.20.0
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-03-26 10:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 14:34 [PATCH] media: davinci-isif: avoid uninitialized variable use Arnd Bergmann
2019-03-22 16:01 ` Nathan Chancellor
2019-03-26 10:23 ` Lad, Prabhakar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).