[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 2/2] linux-user/mmap: Fix Clang 'type-limit-compare' warn
From: |
Aleksandar Markovic |
Subject: |
Re: [RFC PATCH 2/2] linux-user/mmap: Fix Clang 'type-limit-compare' warning |
Date: |
Sun, 3 May 2020 14:55:37 +0200 |
нед, 3. мај 2020. у 14:49 Aleksandar Markovic
<address@hidden> је написао/ла:
>
> нед, 3. мај 2020. у 13:33 Philippe Mathieu-Daudé <address@hidden> је
> написао/ла:
> >
> > When building with Clang 10 on Fedora 32, we get:
> >
> > CC linux-user/mmap.o
> > linux-user/mmap.c:720:49: error: result of comparison 'unsigned long' >
> > 18446744073709551615 is always false
> > [-Werror,-Wtautological-type-limit-compare]
> > if ((unsigned long)host_addr + new_size > (abi_ulong)-1) {
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
> >
> > Fix by restricting the check for when target sizeof(abi_ulong) is
> > smaller than target sizeof(unsigned long).
> >
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> > linux-user/mmap.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> > index e378033797..b14652d894 100644
> > --- a/linux-user/mmap.c
> > +++ b/linux-user/mmap.c
> > @@ -714,6 +714,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> > old_size,
> > errno = ENOMEM;
> > host_addr = MAP_FAILED;
> > }
> > +#if TARGET_ABI_BITS < TARGET_LONG_BITS
Or, for that matter, a comment should be inserted before this
line with explanation why the check is not needed for this case.
I think QEMU is too full with unexplained "ifdefs", which, of
course, doesn't help readibility.
> > /* Check if address fits target address space */
> > if ((unsigned long)host_addr + new_size > (abi_ulong)-1) {
> > /* Revert mremap() changes */
> > @@ -721,6 +722,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> > old_size,
> > errno = ENOMEM;
> > host_addr = MAP_FAILED;
> > }
> > +#endif /* TARGET_ABI_BITS < TARGET_LONG_BITS */
>
> Hm, Philippe, this will silence the clang error, but is this the right
> thing to do?
>
> Why do you think the case:
>
> TARGET_ABI_BITS < TARGET_LONG_BITS
>
> doesn't need this check? In any case, for clarity, the reason should
> be mentioned in the commit message.
>
> Regards,
> Aleksandar
>
>
> > }
> >
> > if (host_addr == MAP_FAILED) {
> > --
> > 2.21.3
> >
> >