Message ID | 20230113162200.3010804-3-berrange@redhat.com |
---|---|
State | New |
Headers | show |
Series | audio: remove deprecated QEMU_AUDIO env support | expand |
Am 13.01.23 um 17:21 schrieb Daniel P. Berrangé: > The audio_calloc function does various checks on the size and > nmembers parameters to detect various error conditions. There > are only 5 callers > > * alsa_poll_helper: the pollfd count is small and bounded, > * audio_pcm_create_voice_pair_: allocating a single fixed > size struct > * audio_pcm_sw_alloc_resources_: samples could be negative > zero, or overflow, so needs a check > * audio_pcm_hw_add_new_: voice size could be zero for > backends that don't support audio input > * st_rate_start: allocating a single fixed size struct > > IOW, only two of the callers need special error checks and > it is clearer if their respective checks are inlined. Thus > audio_calloc can be eliminated. Hi Daniel, my patch series at https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg02895.html also removes audio_calloc(). There will be merge conflicts. With best regards, Volker > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > audio/alsaaudio.c | 6 +----- > audio/audio.c | 20 -------------------- > audio/audio_int.h | 1 - > audio/audio_template.h | 28 ++++++++++++++-------------- > audio/mixeng.c | 7 +------ > tests/qtest/fuzz-sb16-test.c | 6 ++++-- > 6 files changed, 20 insertions(+), 48 deletions(-) > > diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c > index 714bfb6453..5f50dfa0bf 100644 > --- a/audio/alsaaudio.c > +++ b/audio/alsaaudio.c > @@ -222,11 +222,7 @@ static int alsa_poll_helper (snd_pcm_t *handle, struct pollhlp *hlp, int mask) > return -1; > } > > - pfds = audio_calloc ("alsa_poll_helper", count, sizeof (*pfds)); > - if (!pfds) { > - dolog ("Could not initialize poll mode\n"); > - return -1; > - } > + pfds = g_new0(struct pollfd, count); > > err = snd_pcm_poll_descriptors (handle, pfds, count); > if (err < 0) { > diff --git a/audio/audio.c b/audio/audio.c > index 7b4b957945..f397072a1f 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -146,26 +146,6 @@ static inline int audio_bits_to_index (int bits) > } > } > > -void *audio_calloc (const char *funcname, int nmemb, size_t size) > -{ > - int cond; > - size_t len; > - > - len = nmemb * size; > - cond = !nmemb || !size; > - cond |= nmemb < 0; > - cond |= len < size; > - > - if (audio_bug ("audio_calloc", cond)) { > - AUD_log (NULL, "%s passed invalid arguments to audio_calloc\n", > - funcname); > - AUD_log (NULL, "nmemb=%d size=%zu (len=%zu)\n", nmemb, size, len); > - return NULL; > - } > - > - return g_malloc0 (len); > -} > - > void AUD_vlog (const char *cap, const char *fmt, va_list ap) > { > if (cap) { > diff --git a/audio/audio_int.h b/audio/audio_int.h > index e87ce014a0..b0cc2cd390 100644 > --- a/audio/audio_int.h > +++ b/audio/audio_int.h > @@ -251,7 +251,6 @@ void audio_pcm_init_info (struct audio_pcm_info *info, 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_calloc (const char *funcname, int nmemb, size_t size); > > void audio_run(AudioState *s, const char *msg); > > diff --git a/audio/audio_template.h b/audio/audio_template.h > index 720a32e57e..564cbb1f01 100644 > --- a/audio/audio_template.h > +++ b/audio/audio_template.h > @@ -116,13 +116,20 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW *sw) > samples = (int64_t)sw->HWBUF->size * sw->ratio >> 32; > #endif > > - sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample)); > - if (!sw->buf) { > - dolog ("Could not allocate buffer for `%s' (%d samples)\n", > + if (audio_bug(__func__, samples <= 0)) { > + dolog ("Could not allocate buffer for '%s', samples %d <= 0\n", > SW_NAME (sw), samples); > return -1; > } > > + if (audio_bug(__func__, (SIZE_MAX / sizeof(struct st_sample) < samples))) { > + dolog ("Could not allocate buffer for '%s', samples %d overflows\n", > + SW_NAME (sw), samples); > + return -1; > + } > + > + sw->buf = g_new0(struct st_sample, samples); > + > #ifdef DAC > sw->rate = st_rate_start (sw->info.freq, sw->hw->info.freq); > #else > @@ -264,13 +271,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s, > return NULL; > } > > - hw = audio_calloc(__func__, 1, glue(drv->voice_size_, TYPE)); > - if (!hw) { > - dolog ("Can not allocate voice `%s' size %d\n", > - drv->name, glue (drv->voice_size_, TYPE)); > + if (audio_bug(__func__, glue(drv->voice_size_, TYPE) == 0)) { > + dolog ("Voice size is zero"); > return NULL; > } > > + hw = g_malloc0(glue(drv->voice_size_, TYPE)); > hw->s = s; > hw->pcm_ops = drv->pcm_ops; > > @@ -398,12 +404,7 @@ static SW *glue(audio_pcm_create_voice_pair_, TYPE)( > hw_as = *as; > } > > - sw = audio_calloc(__func__, 1, sizeof(*sw)); > - if (!sw) { > - dolog ("Could not allocate soft voice `%s' (%zu bytes)\n", > - sw_name ? sw_name : "unknown", sizeof (*sw)); > - goto err1; > - } > + sw = g_new0(SW, 1); > sw->s = s; > > hw = glue(audio_pcm_hw_add_, TYPE)(s, &hw_as); > @@ -424,7 +425,6 @@ err3: > glue (audio_pcm_hw_gc_, TYPE) (&hw); > err2: > g_free (sw); > -err1: > return NULL; > } > > diff --git a/audio/mixeng.c b/audio/mixeng.c > index 100a306d6f..fe454e0725 100644 > --- a/audio/mixeng.c > +++ b/audio/mixeng.c > @@ -414,12 +414,7 @@ struct rate { > */ > void *st_rate_start (int inrate, int outrate) > { > - struct rate *rate = audio_calloc(__func__, 1, sizeof(*rate)); > - > - if (!rate) { > - dolog ("Could not allocate resampler (%zu bytes)\n", sizeof (*rate)); > - return NULL; > - } > + struct rate *rate = g_new0(struct rate, 1); > > rate->opos = 0; > > diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c > index fc445b1871..a28b93be3a 100644 > --- a/tests/qtest/fuzz-sb16-test.c > +++ b/tests/qtest/fuzz-sb16-test.c > @@ -10,7 +10,8 @@ > #include "libqtest.h" > > /* > - * This used to trigger the assert in audio_calloc > + * This used to trigger the audio_bug calls in > + * audio_pcm_sw_alloc_resources > * https://bugs.launchpad.net/qemu/+bug/1910603 > */ > static void test_fuzz_sb16_0x1c(void) > @@ -38,7 +39,8 @@ static void test_fuzz_sb16_0x91(void) > } > > /* > - * This used to trigger the assert in audio_calloc > + * This used to trigger the audio_bug calls in > + * audio_pcm_sw_alloc_resources > * through command 0xd4 > */ > static void test_fuzz_sb16_0xd4(void)
On Sun, Jan 15, 2023 at 03:03:29PM +0100, Volker Rümelin wrote: > Am 13.01.23 um 17:21 schrieb Daniel P. Berrangé: > > The audio_calloc function does various checks on the size and > > nmembers parameters to detect various error conditions. There > > are only 5 callers > > > > * alsa_poll_helper: the pollfd count is small and bounded, > > * audio_pcm_create_voice_pair_: allocating a single fixed > > size struct > > * audio_pcm_sw_alloc_resources_: samples could be negative > > zero, or overflow, so needs a check > > * audio_pcm_hw_add_new_: voice size could be zero for > > backends that don't support audio input > > * st_rate_start: allocating a single fixed size struct > > > > IOW, only two of the callers need special error checks and > > it is clearer if their respective checks are inlined. Thus > > audio_calloc can be eliminated. > > Hi Daniel, > > my patch series at > https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg02895.html also > removes audio_calloc(). There will be merge conflicts. Ah, yes, sorry I missed that. I've sent a few comments on your impl. Consider this patch dropped. With regards, Daniel
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c index 714bfb6453..5f50dfa0bf 100644 --- a/audio/alsaaudio.c +++ b/audio/alsaaudio.c @@ -222,11 +222,7 @@ static int alsa_poll_helper (snd_pcm_t *handle, struct pollhlp *hlp, int mask) return -1; } - pfds = audio_calloc ("alsa_poll_helper", count, sizeof (*pfds)); - if (!pfds) { - dolog ("Could not initialize poll mode\n"); - return -1; - } + pfds = g_new0(struct pollfd, count); err = snd_pcm_poll_descriptors (handle, pfds, count); if (err < 0) { diff --git a/audio/audio.c b/audio/audio.c index 7b4b957945..f397072a1f 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -146,26 +146,6 @@ static inline int audio_bits_to_index (int bits) } } -void *audio_calloc (const char *funcname, int nmemb, size_t size) -{ - int cond; - size_t len; - - len = nmemb * size; - cond = !nmemb || !size; - cond |= nmemb < 0; - cond |= len < size; - - if (audio_bug ("audio_calloc", cond)) { - AUD_log (NULL, "%s passed invalid arguments to audio_calloc\n", - funcname); - AUD_log (NULL, "nmemb=%d size=%zu (len=%zu)\n", nmemb, size, len); - return NULL; - } - - return g_malloc0 (len); -} - void AUD_vlog (const char *cap, const char *fmt, va_list ap) { if (cap) { diff --git a/audio/audio_int.h b/audio/audio_int.h index e87ce014a0..b0cc2cd390 100644 --- a/audio/audio_int.h +++ b/audio/audio_int.h @@ -251,7 +251,6 @@ void audio_pcm_init_info (struct audio_pcm_info *info, 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_calloc (const char *funcname, int nmemb, size_t size); void audio_run(AudioState *s, const char *msg); diff --git a/audio/audio_template.h b/audio/audio_template.h index 720a32e57e..564cbb1f01 100644 --- a/audio/audio_template.h +++ b/audio/audio_template.h @@ -116,13 +116,20 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW *sw) samples = (int64_t)sw->HWBUF->size * sw->ratio >> 32; #endif - sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample)); - if (!sw->buf) { - dolog ("Could not allocate buffer for `%s' (%d samples)\n", + if (audio_bug(__func__, samples <= 0)) { + dolog ("Could not allocate buffer for '%s', samples %d <= 0\n", SW_NAME (sw), samples); return -1; } + if (audio_bug(__func__, (SIZE_MAX / sizeof(struct st_sample) < samples))) { + dolog ("Could not allocate buffer for '%s', samples %d overflows\n", + SW_NAME (sw), samples); + return -1; + } + + sw->buf = g_new0(struct st_sample, samples); + #ifdef DAC sw->rate = st_rate_start (sw->info.freq, sw->hw->info.freq); #else @@ -264,13 +271,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s, return NULL; } - hw = audio_calloc(__func__, 1, glue(drv->voice_size_, TYPE)); - if (!hw) { - dolog ("Can not allocate voice `%s' size %d\n", - drv->name, glue (drv->voice_size_, TYPE)); + if (audio_bug(__func__, glue(drv->voice_size_, TYPE) == 0)) { + dolog ("Voice size is zero"); return NULL; } + hw = g_malloc0(glue(drv->voice_size_, TYPE)); hw->s = s; hw->pcm_ops = drv->pcm_ops; @@ -398,12 +404,7 @@ static SW *glue(audio_pcm_create_voice_pair_, TYPE)( hw_as = *as; } - sw = audio_calloc(__func__, 1, sizeof(*sw)); - if (!sw) { - dolog ("Could not allocate soft voice `%s' (%zu bytes)\n", - sw_name ? sw_name : "unknown", sizeof (*sw)); - goto err1; - } + sw = g_new0(SW, 1); sw->s = s; hw = glue(audio_pcm_hw_add_, TYPE)(s, &hw_as); @@ -424,7 +425,6 @@ err3: glue (audio_pcm_hw_gc_, TYPE) (&hw); err2: g_free (sw); -err1: return NULL; } diff --git a/audio/mixeng.c b/audio/mixeng.c index 100a306d6f..fe454e0725 100644 --- a/audio/mixeng.c +++ b/audio/mixeng.c @@ -414,12 +414,7 @@ struct rate { */ void *st_rate_start (int inrate, int outrate) { - struct rate *rate = audio_calloc(__func__, 1, sizeof(*rate)); - - if (!rate) { - dolog ("Could not allocate resampler (%zu bytes)\n", sizeof (*rate)); - return NULL; - } + struct rate *rate = g_new0(struct rate, 1); rate->opos = 0; diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c index fc445b1871..a28b93be3a 100644 --- a/tests/qtest/fuzz-sb16-test.c +++ b/tests/qtest/fuzz-sb16-test.c @@ -10,7 +10,8 @@ #include "libqtest.h" /* - * This used to trigger the assert in audio_calloc + * This used to trigger the audio_bug calls in + * audio_pcm_sw_alloc_resources * https://bugs.launchpad.net/qemu/+bug/1910603 */ static void test_fuzz_sb16_0x1c(void) @@ -38,7 +39,8 @@ static void test_fuzz_sb16_0x91(void) } /* - * This used to trigger the assert in audio_calloc + * This used to trigger the audio_bug calls in + * audio_pcm_sw_alloc_resources * through command 0xd4 */ static void test_fuzz_sb16_0xd4(void)
The audio_calloc function does various checks on the size and nmembers parameters to detect various error conditions. There are only 5 callers * alsa_poll_helper: the pollfd count is small and bounded, * audio_pcm_create_voice_pair_: allocating a single fixed size struct * audio_pcm_sw_alloc_resources_: samples could be negative zero, or overflow, so needs a check * audio_pcm_hw_add_new_: voice size could be zero for backends that don't support audio input * st_rate_start: allocating a single fixed size struct IOW, only two of the callers need special error checks and it is clearer if their respective checks are inlined. Thus audio_calloc can be eliminated. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- audio/alsaaudio.c | 6 +----- audio/audio.c | 20 -------------------- audio/audio_int.h | 1 - audio/audio_template.h | 28 ++++++++++++++-------------- audio/mixeng.c | 7 +------ tests/qtest/fuzz-sb16-test.c | 6 ++++-- 6 files changed, 20 insertions(+), 48 deletions(-)