On Fri, 10 Nov 2023 at 09:16, Manos Pitsidianakis
<manos.pitsidiana...@linaro.org> wrote:
>
> Ignoring the return value by accident is easy to miss as a bug. Such a
> bug was spotted by Coverity CID 1523899. Now, future instances of this
> type of bug will produce a warning when using GCC.
>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
> ---
> audio/audio.h | 2 +-
> hw/arm/omap2.c | 8 +++++++-
> hw/input/tsc210x.c | 8 +++++++-
> 3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/audio/audio.h b/audio/audio.h
> index fcc22307be..b78c75962e 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp {
> void AUD_vlog (const char *cap, const char *fmt, va_list ap)
> G_GNUC_PRINTF(2, 0);
> void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
>
> -bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp);
> +bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
> QEMU_WARN_UNUSED_RESULT;
> void AUD_remove_card (QEMUSoundCard *card);
> CaptureVoiceOut *AUD_add_capture(
> AudioState *s,
> diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
> index f170728e7e..59fc061120 100644
> --- a/hw/arm/omap2.c
> +++ b/hw/arm/omap2.c
> @@ -614,7 +614,13 @@ static struct omap_eac_s *omap_eac_init(struct
> omap_target_agent_s *ta,
> s->codec.card.name = g_strdup(current_machine->audiodev);
> s->codec.card.state = audio_state_by_name(s->codec.card.name,
> &error_fatal);
> }
> - AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal);
> + /*
> + * We pass error_fatal so on error QEMU will exit(). But we check the
> + * return value to make the warn_unused_result compiler warning go away.
> + */
> + if (!AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal)) {
> + exit(1);
> + }
This kind of thing is why Coverity's unused-result warning has a
lot of false positives. We shouldn't introduce extra code like
this to work around the fact that the tooling doesn't understand
our error-handling convention (i.e. error_fatal, and the way
that some functions report errors both via the return value and
also via the Error** argument).
thanks
-- PMM
