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

[Lucas De Marchi]

On Tue, Dec 17, 2024 at 08:17:47PM +0100, Kamil Konieczny wrote:
>Hi Lucas,
>On 2024-12-17 at 12:03:49 -0600, Lucas De Marchi wrote:
>> 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 ?
>>
>> because if it's not defined, how are you going to decide which side of
>> them to use?
>>
>> from the tools/  include in kernel, one could do
>>
>> #ifndef __WORDSIZE
>> #define __WORDSIZE (__SIZEOF_LONG__ * 8)
>> #endif
>>
>> another possible solution is checking ULONG_MAX, like in dnf:
>>
>> https://github.com/rpm-software-management/libdnf/pull/1159/files
>>
>> but I think the kernel one is cleaner.
>>
>> Lucas De Marchi
>>
>
>Why not just 'sizeof(1ul) == 8' ?

that would work too

Lucas De Marchi

>Btw we still have problem here when sizeof(1ull) == 4
>(I hope such architectures are out of our interest though)