Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX

[Philippe Mathieu-Daudé]

On 12/24/18 3:16 AM, Zoltán Kővágó wrote:
> Hi Phil,
> 
> On 2018-12-24 00:49, Philippe Mathieu-Daudé wrote:
>> Hi Zoltán,
>>
>> On 12/23/18 9:51 PM, Kővágó, Zoltán wrote:
>>> There's already a MIN and MAX macro in include/qemu/osdep.h, use them
>>> instead.
>>>
>>> Signed-off-by: Kővágó, Zoltán <address@hidden>
>>>
>>> ---
>>>
>>> Changes from v1:
>>> * removed audio_MIN, audio_MAX macros
>>> ---
>> [...]>
>>> diff --git a/audio/audio.h b/audio/audio.h
>>> index ccfef9e10a..bcbe56d639 100644
>>> --- a/audio/audio.h
>>> +++ b/audio/audio.h
>>> @@ -148,23 +148,6 @@ static inline void *advance (void *p, int incr)
>>>      return (d + incr);
>>>  }
>>>  
>>> -#ifdef __GNUC__
>>> -#define audio_MIN(a, b) ( __extension__ ({      \
>>> -    __typeof (a) ta = a;                        \
>>> -    __typeof (b) tb = b;                        \
>>> -    ((ta)>(tb)?(tb):(ta));                      \
>>> -}))
>>> -
>>> -#define audio_MAX(a, b) ( __extension__ ({      \
>>> -    __typeof (a) ta = a;                        \
>>> -    __typeof (b) tb = b;                        \
>>> -    ((ta)<(tb)?(tb):(ta));                      \
>>> -}))
>>> -#else
>>> -#define audio_MIN(a, b) ((a)>(b)?(b):(a))
>>> -#define audio_MAX(a, b) ((a)<(b)?(b):(a))
>>> -#endif
>>> -
>>
>> Those MIN/MAX are smarter than the ones in "qemu/osdep.h", I'd keep them
>> moving them there.
> 
> Yes, but only when using gcc (or clang when it emulates gcc).
> Unfortunately, they work differently when one of the expressions has
> side effects, which is a disaster waiting to happen (when some poor folk
> finally tries to compile it with a non-gcc compiler).

We already use the typeof extension:

$ git grep typeof|wc -l
145

For MIN/MAX I'd use rather use the __auto_type extension.

> Or do we support any compilers beside gcc and clang? Because if not,
> sure, do it, just remove the #ifdef and keep only the gcc version.

Yes, this is checked by the ./configure script:

1850 # Check whether the compiler matches our minimum requirements:
...
1859 #   error You need at least Clang v3.4 to compile QEMU
...
1864 #  error You need at least GCC v4.8 to compile QEMU
...
1867 # error You either need GCC or Clang to compiler QEMU

Regards,

Phil.