Re: [PATCH 3/4] ALSA: timer: Introduce virtual userspace-driven timers

From: Ivan Orlov
Date: Sun Jul 28 2024 - 04:49:15 EST


On 7/28/24 07:52, Christophe JAILLET wrote:
Hi,


Hi Christophe,

...

diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index b970a1734647..3cf82641fc67 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -251,6 +251,17 @@ config SND_JACK_INJECTION_DEBUG
        Say Y if you are debugging via jack injection interface.
        If unsure select "N".
+config SND_UTIMER
+    bool "Enable support for userspace-controlled virtual timers"
+    depends on SND_TIMER
+    help
+      Say Y to enable the support of userspace-controlled timers. These
+      timers are purely virtual, and they are supposed to be triggered
+      from userspace. They could be quite useful when synchronizing the
+      sound timing with userspace applications (for instance, when sending
+      data through snd-aloop).
+

Unneeded extra new line.

+
  config SND_VMASTER
      bool

...

+static void snd_utimer_free(struct snd_utimer *utimer)
+{
+    snd_timer_free(utimer->timer);
+    snd_utimer_put_id(utimer);

Missing kfree(utimer->name); ?


Yeah, it definitely should be here... Thank you for finding this!

+    kfree(utimer);
+}

...

+static int snd_utimer_create(struct snd_utimer_info *utimer_info,
+                 struct snd_utimer **r_utimer)
+{
+    struct snd_utimer *utimer;
+    struct snd_timer *timer;
+    struct snd_timer_id tid;
+    int utimer_id;
+    int err = 0;
+    char *timer_name;
+
+    utimer = kzalloc(sizeof(*utimer), GFP_KERNEL);
+    if (!utimer)
+        return -ENOMEM;
+
+    timer_name = kzalloc(SNDRV_UTIMER_NAME_LEN, GFP_KERNEL);

kasprintf(GFP_KERNEL, "snd-utimer%d", utimer_id); ?
and SNDRV_UTIMER_NAME_LEN becomes useless too.

In snd_timer_new() it is copied in a char[64] anyway, and if utimer_id is small, we could even save a few bytes of memory.


Wow, cool, I haven't heard of kasprintf but now I'll use it here in V2. Thanks!

--
Kind regards,
Ivan Orlov