[PATCH i-g-t] lib/igt_core: Use suffix for casting to long to avoid overflow

[Zbigniew Kempczyński]

On Tue, Dec 17, 2024 at 05:47:44PM +0100, Kamil Konieczny wrote:
> Hi Zbigniew,
> On 2024-12-17 at 11:58:07 +0100, Zbigniew Kempczyński wrote:
> 
> please improve subject, imho better something like:
> 
> Avoid overflow in micro/nano-seconds calculations
> or
> Fix overflow in micro/nano-seconds calculations
> 
> or propose yourself one line description.
> 
> > Multiplication without using casting to long (long) for at least
> > one argument in expression may cause overflow especially when we're
> > using this for calculating timeouts in nanoseconds precision.
> > 
> > Ensure we're picking valid ul/ull suffix for base definitions for
> > all dependents.
> > 
> > Reported-by: Ilia Levi <ilia.levi at intel.com>
> > Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/163
> > 
> 
> I agree here with Lucas, remove this empty line.
> 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Cc: Ilia Levi <ilia.levi at intel.com>
> > ---
> >  lib/igt_core.h | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/igt_core.h b/lib/igt_core.h
> > index 90f57402f0..1b6e70182b 100644
> > --- a/lib/igt_core.h
> > +++ b/lib/igt_core.h
> > @@ -1517,11 +1517,17 @@ void igt_kmsg(const char *format, ...);
> >  #define le32_to_cpu(x)  (x)
> >  #endif
> >  
> > -#define MSEC_PER_SEC (1000u)
> > +#if __WORDSIZE == 64
> 
> I am not sure if this is defined on all platforms where igt
> is compiled, on musl and on non-Linux like FreeBSD,
> why not make #ifndef/#define/#endif for __WORDSIZE ?

I think it would be good to add this to build system, I base on
results on:

https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1331414

We already rely on __WORDSIZE in other places in the code (PRIx64)
so I've assumed it's safe to use it here.

--
Zbigniew

> 
> BTW is it just sizeof(1ul) == 8 ?
> 
> With this fixed and subject improved
> Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> 
> Regards,
> Kamil
> 
> > +#define MSEC_PER_SEC (1000ul)
> > +#define USEC_PER_MSEC (1000ul)
> > +#else
> > +#define MSEC_PER_SEC (1000ull)
> > +#define USEC_PER_MSEC (1000ull)
> > +#endif
> > +
> >  #define USEC_PER_SEC (1000u * MSEC_PER_SEC)
> >  #define USEC_PER_DECISEC (100u * MSEC_PER_SEC)
> >  #define NSEC_PER_SEC (1000u * USEC_PER_SEC)
> > -#define USEC_PER_MSEC (1000u)
> >  #define NSEC_PER_MSEC (1000u * USEC_PER_MSEC)
> >  
> >  #define for_if(expr__) if (!(expr__)) {} else
> > -- 
> > 2.34.1
> >