Hi

On Tue, Feb 24, 2026 at 11:30 AM Peter Maydell <[email protected]> wrote:
>
> On Mon, 23 Feb 2026 at 13:51, <[email protected]> wrote:
> >
> > From: Marc-André Lureau <[email protected]>
> >
> > audio_bug() is a bit unconventional, it is meant to be used as a
> > condition expression, passing the actual condition as second argument
> > (and __func__ as first argument).
> >
> > If the condition is true, it uses AUD_log() to print to stderr, and has
> > some dubious recommendations printed only once.
> >
> > This change:
> > - clears the control flow, and make the condition directly visible in
> >   the 'if' statement.
> > - uses standard QEMU error_report()
> > - audio_bug() now captures __func__
> > - remove the "Save all your work and restart..." once hint
> >
> > Reviewed-by: Mark Cave-Ayland <[email protected]>
> > Reviewed-by: Akihiko Odaki <[email protected]>
> > Signed-off-by: Marc-André Lureau <[email protected]>
>
> Hi; Coverity points out a bug here (CID 1645326, 1645328);
> this looks like it is probably a pre-existing one that was either
> re-noticed because of the code changes in this area, or else
> made clearer to Coverity because of the cleanup of the control
> flow logic. Anyway:
>
> > @@ -498,9 +498,9 @@ static SW *glue(audio_mixeng_backend_open_, TYPE) (
> >      AudioMixengBackendClass *k;
> >      AudiodevPerDirectionOptions *pdo;
> >
> > -    if (audio_bug(__func__, !be || !name || !callback_fn || !as)) {
> > -        dolog("backend=%p name=%p callback_fn=%p as=%p\n",
> > -              be, name, callback_fn, as);
> > +    if (!be || !name || !callback_fn || !as) {
> > +        audio_bug("backend=%p name=%p callback_fn=%p as=%p",
> > +                  be, name, callback_fn, as);
> >          goto fail;
> >      }
>
> In this error-exit check, we will go to the "fail" label if be is NULL.
> But the code at 'fail:' does:
>
>     glue(audio_be_close_, TYPE)(be, sw);
>
> and those functions will crash if passed a NULL AudioBackend pointer.
>
> Presumably we should simply return in the be == NULL case ?

I think so. abort() may be an option too imho

Do you want me to send a patch?

-- 
Marc-André Lureau

Reply via email to