Re: [PATCH v1 1/4] virtio-snd: add virtio_snd_is_config_valid()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Manos,

On 22/4/24 14:52, Manos Pitsidianakis wrote:
> Factor out virtio_snd_config value validation in a separate function, in
> order to re-use it in follow up commits.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>   hw/audio/virtio-snd.c | 47 ++++++++++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
> index c80b58bf5d..7ca9ed251c 100644
> --- a/hw/audio/virtio-snd.c
> +++ b/hw/audio/virtio-snd.c
> @@ -1045,6 +1045,34 @@ virtio_snd_vm_state_change(void *opaque, bool running,
>       }
>   }
>   
> +static bool
> +virtio_snd_is_config_valid(virtio_snd_config snd_conf, Error **errp)
> +{
> +    if (snd_conf.jacks > 8) {
> +        error_setg(errp,
> +                   "Invalid number of jacks: %"PRIu32
> +                   ": maximum value is 8", snd_conf.jacks);
> +        return false;
> +    }
> +    if (snd_conf.streams < 1 || snd_conf.streams > 64) {

Why the undocumented 10 -> 64 change?

> +        error_setg(errp,
> +                   "Invalid number of streams: %"PRIu32
> +                   ": minimum value is 1, maximum value is 64",
> +                   snd_conf.streams);
> +        return false;
> +    }
> +
> +    if (snd_conf.chmaps > VIRTIO_SND_CHMAP_MAX_SIZE) {
> +        error_setg(errp,
> +                  "Invalid number of channel maps: %"PRIu32
> +                  ": VIRTIO v1.2 sets the maximum value at %"PRIu32,
> +                  snd_conf.chmaps, VIRTIO_SND_CHMAP_MAX_SIZE);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>   static void virtio_snd_realize(DeviceState *dev, Error **errp)
>   {
>       ERRP_GUARD();
> @@ -1055,24 +1083,7 @@ static void virtio_snd_realize(DeviceState *dev, Error **errp)
>   
>       trace_virtio_snd_realize(vsnd);
>   
> -    /* check number of jacks and streams */
> -    if (vsnd->snd_conf.jacks > 8) {
> -        error_setg(errp,
> -                   "Invalid number of jacks: %"PRIu32,
> -                   vsnd->snd_conf.jacks);
> -        return;
> -    }
> -    if (vsnd->snd_conf.streams < 1 || vsnd->snd_conf.streams > 10) {
> -        error_setg(errp,
> -                   "Invalid number of streams: %"PRIu32,
> -                    vsnd->snd_conf.streams);
> -        return;
> -    }
> -
> -    if (vsnd->snd_conf.chmaps > VIRTIO_SND_CHMAP_MAX_SIZE) {
> -        error_setg(errp,
> -                   "Invalid number of channel maps: %"PRIu32,
> -                   vsnd->snd_conf.chmaps);
> +    if (!virtio_snd_is_config_valid(vsnd->snd_conf, errp)) {
>           return;
>       }
>