qemu-trivial
[Top][All Lists]
Advanced

[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
> >
> >



reply via email to

[Prev in Thread] Current Thread [Next in Thread]