Re: [RFC PATCH v2 01/78] include/qemu/compiler.h: replace QEMU_FALLTHROUGH with fallthrough

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 18 Oct 2023 at 14:12, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:
>>
>> > On Wed, 18 Oct 2023 13:32, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >>> diff --git a/audio/pwaudio.c b/audio/pwaudio.c
>> >>> index 3ce5f6507b..bf26fadb06 100644
>> >>> --- a/audio/pwaudio.c
>> >>> +++ b/audio/pwaudio.c
>> >>> @@ -1,29 +1,29 @@
>> >>>  /*
>> >>>   * QEMU PipeWire audio driver
>> >>>   *
>> >>>   * Copyright (c) 2023 Red Hat Inc.
>> >>>   *
>> >>>   * Author: Dorinda Bassey       <dbassey@redhat.com>
>> >>>   *
>> >>>   * SPDX-License-Identifier: GPL-2.0-or-later
>> >>>   */
>> >>>  +#include <spa/param/audio/format-utils.h>
>> >>> +#include <spa/utils/ringbuffer.h>
>> >>> +#include <spa/utils/result.h>
>> >>> +#include <spa/param/props.h>
>> >>>  #include "qemu/osdep.h"
>> >>>  #include "qemu/module.h"
>> >>>  #include "audio.h"
>> >>>  #include <errno.h>
>> >>>  #include "qemu/error-report.h"
>> >>>  #include "qapi/error.h"
>> >>> -#include <spa/param/audio/format-utils.h>
>> >>> -#include <spa/utils/ringbuffer.h>
>> >>> -#include <spa/utils/result.h>
>> >>> -#include <spa/param/props.h>
>> >>
>> >>Was this an autofmt change sneaking in? osdep.h should always be first
>> >>per style.rst.
>> >
>> > It should have been mentioned in the commit message and in a code
>> > comment. libspa throws errors because the preprocessor changes
>> > `fallthrough` to our macro definition. I do not like the reordering.
>> > My other thought was to undef fallthrough and re-include
>> > "qemu/compiler.h" after the libspa includes.
>>
>> Ahh this stuff:
>>
>>   #if defined(__clang__) && defined(__cplusplus) && __cplusplus >= 201103L
>>      /* clang's fallthrough annotations are only available starting in C++11. */
>>   #  define SPA_FALLTHROUGH [[clang::fallthrough]];
>>   #elif __GNUC__ >= 7 || __clang_major__ >= 10
>>   #  define SPA_FALLTHROUGH __attribute__ ((fallthrough));
>>   #else
>>   #  define SPA_FALLTHROUGH /* FALLTHROUGH */
>>   #endif
>>
>> I think this is papering over a potential problem we might have with
>> multiple libraries and probably an argument in favour of an explicit
>> QEMU_FALLTHROUGH macro to avoid the attribute clash.
>
> Is there a reason this thread lost the qemu-devel cc ?

Not deliberately. I think my mail clients objected to open list:ARM SMMU
<qemu-arm@nongnu.org> and skipped the last two CC's and I didn't notice.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro