Re: [PATCH 12/18] media: staging: atomisp: avoid a warning if 32 bits build

[Mauro Carvalho Chehab]

Em Wed, 28 Mar 2018 17:13:29 +0300
Dan Carpenter  escreveu:

> On Mon, Mar 26, 2018 at 05:10:45PM -0400, Mauro Carvalho Chehab wrote:
> > Checking if a size_t value is bigger than ULONG_INT only makes
> > sense if building on 64 bits, as warned by:
> > 
> > drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:697
> >  gmin_get_config_var() warn: impossible condition '(*out_len > (~0)) => 
> > (0-u32max > u32max)'
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 
> > 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git 
> > a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
> > b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> > index be0c5e11e86b..3283c1b05d6a 100644
> > --- 
> > a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> > +++ 
> > b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> > @@ -693,9 +693,11 @@ static int gmin_get_config_var(struct device *dev, 
> > const char *var,
> > for (i = 0; i < sizeof(var8) && var8[i]; i++)
> > var16[i] = var8[i];
> >  
> > +#ifdef CONFIG_64BIT
> > /* To avoid owerflows when calling the efivar API */
> > if (*out_len > ULONG_MAX)
> > return -EINVAL;
> > +#endif  
> 
> I should just silence this particular warning in Smatch.  I feel like
> this is a pretty common thing and the ifdefs aren't very pretty.  :(

Smatch actually warned about a real thing here: atomisp is
doing a check in 32bits that it is always true. So, IMO,
something is needed to prevent 32bits extra useless code somehow,
perhaps via some EFI-var specific function that would do nothing
on 32 bits.

That's the first time I noticed this code on media (although I might
have missed something), so I guess this kind of checking is actually
not that common.

Regards,
Mauro

Re: [PATCH 12/18] media: staging: atomisp: avoid a warning if 32 bits build

[Dan Carpenter]

On Mon, Mar 26, 2018 at 05:10:45PM -0400, Mauro Carvalho Chehab wrote:
> Checking if a size_t value is bigger than ULONG_INT only makes
> sense if building on 64 bits, as warned by:
>   
> drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:697 
> gmin_get_config_var() warn: impossible condition '(*out_len > (~0)) => 
> (0-u32max > u32max)'
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 
> ++
>  1 file changed, 2 insertions(+)
> 
> diff --git 
> a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
> b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> index be0c5e11e86b..3283c1b05d6a 100644
> --- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
> @@ -693,9 +693,11 @@ static int gmin_get_config_var(struct device *dev, const 
> char *var,
>   for (i = 0; i < sizeof(var8) && var8[i]; i++)
>   var16[i] = var8[i];
>  
> +#ifdef CONFIG_64BIT
>   /* To avoid owerflows when calling the efivar API */
>   if (*out_len > ULONG_MAX)
>   return -EINVAL;
> +#endif

I should just silence this particular warning in Smatch.  I feel like
this is a pretty common thing and the ifdefs aren't very pretty.  :(

regards,
dan carpenter

[PATCH 12/18] media: staging: atomisp: avoid a warning if 32 bits build

[Mauro Carvalho Chehab]

Checking if a size_t value is bigger than ULONG_INT only makes
sense if building on 64 bits, as warned by:

drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:697 
gmin_get_config_var() warn: impossible condition '(*out_len > (~0)) => 
(0-u32max > u32max)'

Signed-off-by: Mauro Carvalho Chehab 
---
 .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 ++
 1 file changed, 2 insertions(+)

diff --git 
a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c 
b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index be0c5e11e86b..3283c1b05d6a 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -693,9 +693,11 @@ static int gmin_get_config_var(struct device *dev, const 
char *var,
for (i = 0; i < sizeof(var8) && var8[i]; i++)
var16[i] = var8[i];
 
+#ifdef CONFIG_64BIT
/* To avoid owerflows when calling the efivar API */
if (*out_len > ULONG_MAX)
return -EINVAL;
+#endif
 
/* Not sure this API usage is kosher; efivar_entry_get()'s
 * implementation simply uses VariableName and VendorGuid from
-- 
2.14.3