From: Marc-André Lureau <marcandre.lureau@redhat.com>

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 <mark.caveayland@nutanix.com>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 audio/audio_int.h       |  2 --
 audio/audio_template.h  | 38 +++++++++++------------
 audio/audio-mixeng-be.c | 69 ++++++++++++++++-------------------------
 3 files changed, 46 insertions(+), 63 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index e0ed6af67ed..20ec7883aab 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -244,8 +244,6 @@ extern const char *audio_prio_list[];
 void audio_pcm_init_info (struct audio_pcm_info *info, const struct audsettings *as);
 void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len);
 
-int audio_bug (const char *funcname, int cond);
-
 void audio_run(AudioMixengBackend *s, const char *msg);
 
 typedef struct RateCtl {
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 5711f4a26e1..9dde36738bc 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -64,15 +64,15 @@ static void glue(audio_init_nb_voices_, TYPE)(AudioMixengBackend *s,
                min_voices);
     }
 
-    if (audio_bug(__func__, !voice_size && max_voices)) {
-        dolog("drv=`%s' voice_size=0 max_voices=%d\n",
-               k->name, max_voices);
+    if (!voice_size && max_voices) {
+        audio_bug("drv=`%s' voice_size=0 max_voices=%d",
+                  k->name, max_voices);
         glue(s->nb_hw_voices_, TYPE) = 0;
     }
 
-    if (audio_bug(__func__, voice_size && !max_voices)) {
-        dolog("drv=`%s' voice_size=%zu max_voices=0\n",
-              k->name, voice_size);
+    if (voice_size && !max_voices) {
+        audio_bug("drv=`%s' voice_size=%zu max_voices=0",
+                  k->name, voice_size);
     }
 }
 
@@ -88,8 +88,8 @@ static void glue(audio_pcm_hw_alloc_resources_, TYPE)(HW *hw)
 {
     if (glue(audio_get_pdo_, TYPE)(hw->s->dev)->mixing_engine) {
         size_t samples = hw->samples;
-        if (audio_bug(__func__, samples == 0)) {
-            dolog("Attempted to allocate empty buffer\n");
+        if (samples == 0) {
+            audio_bug("Attempted to allocate empty buffer");
         }
 
         HWBUF.buffer = g_new0(st_sample, samples);
@@ -275,8 +275,8 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioMixengBackend *s,
         return NULL;
     }
 
-    if (audio_bug(__func__, !glue(k->init_, TYPE))) {
-        dolog("No host audio driver or missing init_%s\n", NAME);
+    if (!glue(k->init_, TYPE)) {
+        audio_bug("No host audio driver or missing init_%s", NAME);
         return NULL;
     }
 
@@ -295,8 +295,8 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioMixengBackend *s,
         goto err0;
     }
 
-    if (audio_bug(__func__, hw->samples <= 0)) {
-        dolog("hw->samples=%zd\n", hw->samples);
+    if (hw->samples <= 0) {
+        audio_bug("hw->samples=%zd", hw->samples);
         goto err1;
     }
 
@@ -477,8 +477,8 @@ static void glue (audio_close_, TYPE) (SW *sw)
 static void glue(audio_mixeng_backend_close_, TYPE)(AudioBackend *be, SW *sw)
 {
     if (sw) {
-        if (audio_bug(__func__, !be)) {
-            dolog("backend=%p\n", be);
+        if (!be) {
+            audio_bug("backend=%p", be);
             return;
         }
 
@@ -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;
     }
 
@@ -519,8 +519,8 @@ static SW *glue(audio_mixeng_backend_open_, TYPE) (
         goto fail;
     }
 
-    if (audio_bug(__func__, !glue(k->init_, TYPE))) {
-        dolog("Can not open `%s' (no host audio driver)\n", name);
+    if (!glue(k->init_, TYPE)) {
+        error_report("audio: Can not open `%s' (no host audio driver)", name);
         goto fail;
     }
 
diff --git a/audio/audio-mixeng-be.c b/audio/audio-mixeng-be.c
index bb8c0b56f78..45bcfb74338 100644
--- a/audio/audio-mixeng-be.c
+++ b/audio/audio-mixeng-be.c
@@ -31,6 +31,8 @@
 
 #define SW_NAME(sw) (sw)->name ? (sw)->name : "unknown"
 
+#define audio_bug(fmt, ...) error_report("%s: " fmt, __func__, ##__VA_ARGS__)
+
 const struct mixeng_volume nominal_volume = {
     .mute = 0,
 #ifdef FLOAT_MIXENG
@@ -42,23 +44,6 @@ const struct mixeng_volume nominal_volume = {
 #endif
 };
 
-int audio_bug (const char *funcname, int cond)
-{
-    if (cond) {
-        static int shown;
-
-        AUD_log (NULL, "A bug was just triggered in %s\n", funcname);
-        if (!shown) {
-            shown = 1;
-            AUD_log (NULL, "Save all your work and restart without audio\n");
-            AUD_log (NULL, "I am sorry\n");
-        }
-        AUD_log (NULL, "Context:\n");
-    }
-
-    return cond;
-}
-
 /*
  * Convert audio format to mixeng_clip index. Used by audio_pcm_sw_init_ and
  * audio_mixeng_backend_add_capture()
@@ -332,8 +317,8 @@ static size_t audio_pcm_hw_find_min_in (HWVoiceIn *hw)
 static size_t audio_pcm_hw_get_live_in(HWVoiceIn *hw)
 {
     size_t live = hw->total_samples_captured - audio_pcm_hw_find_min_in (hw);
-    if (audio_bug(__func__, live > hw->conv_buf.size)) {
-        dolog("live=%zu hw->conv_buf.size=%zu\n", live, hw->conv_buf.size);
+    if (live > hw->conv_buf.size) {
+        audio_bug("live=%zu hw->conv_buf.size=%zu", live, hw->conv_buf.size);
         return 0;
     }
     return live;
@@ -402,8 +387,8 @@ static size_t audio_pcm_sw_read(SWVoiceIn *sw, void *buf, size_t buf_len)
     if (!live) {
         return 0;
     }
-    if (audio_bug(__func__, live > hw->conv_buf.size)) {
-        dolog("live_in=%zu hw->conv_buf.size=%zu\n", live, hw->conv_buf.size);
+    if (live > hw->conv_buf.size) {
+        audio_bug("live=%zu hw->conv_buf.size=%zu", live, hw->conv_buf.size);
         return 0;
     }
 
@@ -454,8 +439,8 @@ static size_t audio_pcm_hw_get_live_out (HWVoiceOut *hw, int *nb_live)
     if (nb_live1) {
         size_t live = smin;
 
-        if (audio_bug(__func__, live > hw->mix_buf.size)) {
-            dolog("live=%zu hw->mix_buf.size=%zu\n", live, hw->mix_buf.size);
+        if (live > hw->mix_buf.size) {
+            audio_bug("live=%zu hw->mix_buf.size=%zu", live, hw->mix_buf.size);
             return 0;
         }
         return live;
@@ -533,8 +518,8 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t buf_len)
     size_t frames_in_max, frames_out_max, total_in, total_out;
 
     live = sw->total_hw_samples_mixed;
-    if (audio_bug(__func__, live > hw->mix_buf.size)) {
-        dolog("live=%zu hw->mix_buf.size=%zu\n", live, hw->mix_buf.size);
+    if (live > hw->mix_buf.size) {
+        audio_bug("live=%zu hw->mix_buf.size=%zu", live, hw->mix_buf.size);
         return 0;
     }
 
@@ -824,9 +809,9 @@ static size_t audio_get_avail(SWVoiceIn *sw)
     }
 
     live = sw->hw->total_samples_captured - sw->total_hw_samples_acquired;
-    if (audio_bug(__func__, live > sw->hw->conv_buf.size)) {
-        dolog("live=%zu sw->hw->conv_buf.size=%zu\n", live,
-              sw->hw->conv_buf.size);
+    if (live > sw->hw->conv_buf.size) {
+        audio_bug("live=%zu sw->hw->conv_buf.size=%zu", live,
+                  sw->hw->conv_buf.size);
         return 0;
     }
 
@@ -845,9 +830,9 @@ static size_t audio_get_free(SWVoiceOut *sw)
 
     live = sw->total_hw_samples_mixed;
 
-    if (audio_bug(__func__, live > sw->hw->mix_buf.size)) {
-        dolog("live=%zu sw->hw->mix_buf.size=%zu\n", live,
-              sw->hw->mix_buf.size);
+    if (live > sw->hw->mix_buf.size) {
+        audio_bug("live=%zu sw->hw->mix_buf.size=%zu", live,
+                  sw->hw->mix_buf.size);
         return 0;
     }
 
@@ -1002,8 +987,8 @@ static void audio_run_out(AudioMixengBackend *s)
             live = 0;
         }
 
-        if (audio_bug(__func__, live > hw->mix_buf.size)) {
-            dolog("live=%zu hw->mix_buf.size=%zu\n", live, hw->mix_buf.size);
+        if (live > hw->mix_buf.size) {
+            audio_bug("live=%zu hw->mix_buf.size=%zu", live, hw->mix_buf.size);
             continue;
         }
 
@@ -1033,9 +1018,9 @@ static void audio_run_out(AudioMixengBackend *s)
         prev_rpos = hw->mix_buf.pos;
         played = audio_pcm_hw_run_out(hw, live);
         replay_audio_out(&played);
-        if (audio_bug(__func__, hw->mix_buf.pos >= hw->mix_buf.size)) {
-            dolog("hw->mix_buf.pos=%zu hw->mix_buf.size=%zu played=%zu\n",
-                  hw->mix_buf.pos, hw->mix_buf.size, played);
+        if (hw->mix_buf.pos >= hw->mix_buf.size) {
+            audio_bug("hw->mix_buf.pos=%zu hw->mix_buf.size=%zu played=%zu",
+                      hw->mix_buf.pos, hw->mix_buf.size, played);
             hw->mix_buf.pos = 0;
         }
 
@@ -1050,9 +1035,9 @@ static void audio_run_out(AudioMixengBackend *s)
                 continue;
             }
 
-            if (audio_bug(__func__, played > sw->total_hw_samples_mixed)) {
-                dolog("played=%zu sw->total_hw_samples_mixed=%zu\n",
-                      played, sw->total_hw_samples_mixed);
+            if (played > sw->total_hw_samples_mixed) {
+                audio_bug("played=%zu sw->total_hw_samples_mixed=%zu",
+                          played, sw->total_hw_samples_mixed);
                 played = sw->total_hw_samples_mixed;
             }
 
@@ -1195,9 +1180,9 @@ static void audio_run_capture(AudioMixengBackend *s)
                 continue;
             }
 
-            if (audio_bug(__func__, captured > sw->total_hw_samples_mixed)) {
-                dolog("captured=%zu sw->total_hw_samples_mixed=%zu\n",
-                      captured, sw->total_hw_samples_mixed);
+            if (captured > sw->total_hw_samples_mixed) {
+                audio_bug("captured=%zu sw->total_hw_samples_mixed=%zu",
+                          captured, sw->total_hw_samples_mixed);
                 captured = sw->total_hw_samples_mixed;
             }
 
-- 
2.53.0

On Mon, 23 Feb 2026 at 13:51, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> 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 <mark.caveayland@nutanix.com>
> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

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 ?

thanks
-- PMM

Hi

On Tue, Feb 24, 2026 at 11:30 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 23 Feb 2026 at 13:51, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > 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 <mark.caveayland@nutanix.com>
> > Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> 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

On Tue, 24 Feb 2026 at 11:12, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Tue, Feb 24, 2026 at 11:30 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Mon, 23 Feb 2026 at 13:51, <marcandre.lureau@redhat.com> wrote:
> > >
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > @@ -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

If it's a "can't happen unless programming bug" case, than
maybe "assert(be)".

Am I right in reading the code that we only call this function from
audio_be_open_in() and audio_be_open_out() ? Those two callsites
already assert() that name, callback_fn and as are all non-NULL.
So perhaps we should assert(be) there also, and drop this if(...)
check in this function entirely as redundant ?

> Do you want me to send a patch?

Yes, please.

thanks
-- PMM