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

[Ivan Orlov]

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