diff mbox series

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

Message ID 491c651b075ac51f1c54f561bebaaac6dfc7f8de.1713789200.git.manos.pitsidianakis@linaro.org
State New
Headers show
Series virtio_snd_set_config: Fix #2296 | expand

Commit Message

Manos Pitsidianakis April 22, 2024, 12:52 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé April 22, 2024, 1:20 p.m. UTC | #1
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;
>       }
>
diff mbox series

Patch

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) {
+        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;
     }