[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX
From: |
Kővágó Zoltán |
Subject: |
Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX |
Date: |
Thu, 27 Dec 2018 13:49:43 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-12-25 11:40, Philippe Mathieu-Daudé wrote:
> On 12/24/18 9:48 PM, Kővágó Zoltán wrote:
>> On 2018-12-24 18:16, Philippe Mathieu-Daudé wrote:
>>> 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
>>
>> Fair enough. I've tried to check the documentation for supported
>> compilers, but I haven't found any info. Well, I didn't look at the
>> configure script, I admit that.
>
> You haven't found any info because the minimum requirements are not well
> documented on the wiki.
>
>>
>> This is what I came up with:
>>
>> #ifndef MIN
>> #define MIN(a, b) ( __extension__ ({ \
>> __auto_type _a = (a); \
>> __auto_type _b = (b); \
>> _a > _b ? _b : _a; \
>> }))
>> #endif
>> #ifndef MAX
>> #define MAX(a, b) ( __extension__ ({ \
>> __auto_type _a = (a); \
>> __auto_type _b = (b); \
>> _a < _b ? _b : _a; \
>> }))
>> #endif
>>
>>
>> I changed it a bit, since a and b are the macro parameters, so they are
>> the variables that we should put into parentheses, and I renamed ta/tb
>> to _a/_b, this way they're less likely to clobber some other variables.
>
> Sounds good. Now the parentheses around a/b are not necessary.
>
> I wonder however if it wouldn't be cleaner to previously add:
>
> #ifdef MIN
> # undef MIN
> #endif
>
> and for MAX, rather than only use these macros if undefs.
Um, this broke a few things. It also means until now in some cases it
used some random definition of MIN/MAX instead of the one in the header.
/home/dirty_ice/qemu/include/qemu/osdep.h:240:5: error: ‘__auto_type’
used with a bit-field initializer
/home/dirty_ice/qemu/include/qemu/osdep.h:247:35: error: braced-group
within expression allowed only inside a function
The first one is fixable with an explicit cast (ugly but works), but the
second one is more problematic. It means we can't write stuff like
USBPort uports[MAX(MAXPORTS_2, MAXPORTS_3)];
when not in a function. So we either need a dumb version of MIN/MAX, or
scrape the idea altogether.
>
>> To be honest, we could probably drop the __extension__ keyword too,
>> we're not compiling with -pedantic. Other than audio_MIN and audio_MAX,
>> it only appears in the DO_UPCAST macro.
>
> Fine with me!
>
> If you have this patch ready, you can send it alone out of this series,
> no need to resend this whole series for this patch.
There will be adjustmets to other patches too, so I think I'll just
consolidate them.
Regards,
Zoltan
- [Qemu-devel] [PATCH v2 28/52] audio: api for mixeng code free backends, (continued)
- [Qemu-devel] [PATCH v2 28/52] audio: api for mixeng code free backends, Kővágó, Zoltán, 2018/12/23
- [Qemu-devel] [PATCH v2 27/52] audio: use size_t where makes sense, Kővágó, Zoltán, 2018/12/23
- [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX, Kővágó, Zoltán, 2018/12/23
- Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX, Philippe Mathieu-Daudé, 2018/12/23
- Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX, Zoltán Kővágó, 2018/12/23
- Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX, Philippe Mathieu-Daudé, 2018/12/24
- Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX, Kővágó Zoltán, 2018/12/24
- Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX, Philippe Mathieu-Daudé, 2018/12/25
- Re: [Qemu-devel] [PATCH v2 23/52] audio: remove audio_MIN, audio_MAX,
Kővágó Zoltán <=
[Qemu-devel] [PATCH v2 36/52] spiceaudio: port to the new audio backend api, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 32/52] noaudio: port to the new audio backend api, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 31/52] dsoundaudio: port to the new audio backend api, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 33/52] ossaudio: port to the new audio backend api, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 39/52] audio: unify input and output mixeng buffer management, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 30/52] coreaudio: port to the new audio backend api, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 40/52] audio: remove hw->samples, buffer_size_in/out pcm_ops, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 44/52] audio: make mixeng optional, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 46/52] audio: support more than two channels in volume setting, Kővágó, Zoltán, 2018/12/23
[Qemu-devel] [PATCH v2 48/52] audio: basic support for multichannel audio, Kővágó, Zoltán, 2018/12/23