Re: [PATCH 12/18] media: staging: atomisp: avoid a warning if 32 bits build
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/18] media: staging: atomisp: avoid a warning if 32 bits build
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/18] media: staging: atomisp: avoid a warning if 32 bits build
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel