Re: [PATCH v3] osdep: Make MIN/MAX evaluate arguments only once

[Eric Blake]

On 6/2/20 9:29 PM, Eric Blake wrote:

> > > +++ b/accel/tcg/translate-all.c
> > > @@ -2565,9 +2565,9 @@ int page_check_range(target_ulong start, 
> > > target_ulong len, int flags)
> > >       /* This function should never be called with addresses outside the
> > >          guest address space.  If this assert fires, it probably 
> > > indicates
> > >          a missing call to h2g_valid.  */
> > > -#if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
> > > -    assert(start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
> > > -#endif
> > > +    if (TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS) {
> > > +        assert(start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
> > > +    }
> >
> > IIRC the ifdef is required for clang warnings vs the shift.
> > Have you tested that?
>
> I have not yet tested with clang.  We'll see if the CLI bots get to that 
> before I do...  But if clang isn't happy, I may have to introduce yet a 
> third macro, MIN_PP, safe for use in preprocessor statements.

I've now run a clang build over the entire tree (using clang 10.0.0 from 
Fedora 32, which required other pending patches mentioned on the list to 
work around unrelated warnings), the entire tree built without issue. 
So at least one version of clang compiled my rewrite of this hunk just 
fine, although it does not rule out what older versions might do.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org