Re: [PATCH v3 12/12] ALSA: hda/cs35l56: Add driver for Cirrus Logic CS35L56 amplifier

[Richard Fitzgerald]

On 30/05/2023 08:39, Claudiu.Beznea@xxxxxxxxxxxxx wrote:
> On 26.05.2023 15:28, Richard Fitzgerald wrote:
> > +static int cs35l56_hda_request_firmware_file(struct cs35l56_hda *cs35l56,
> > +                                            const struct firmware **firmware, char **filename,
> > +                                            const char *dir, const char *system_name,
> > +                                            const char *amp_name,
> > +                                            const char *filetype)
> > +{
> > +       char *s, c;
> > +       int ret = 0;
> > +
> > +       if (system_name && amp_name)
> > +               *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc-%s-%s.%s", dir,
> > +                                     cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
> > +                                     system_name, amp_name, filetype);
> > +       else if (system_name)
> > +               *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc-%s.%s", dir,
> > +                                     cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
> > +                                     system_name, filetype);
> > +       else
> > +               *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc.%s", dir,
> > +                                     cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
> > +                                     filetype);
> > +
> > +       if (!*filename)
> > +               return -ENOMEM;
> > +
> > +       /*
> > +        * Make sure that filename is lower-case and any non alpha-numeric
> > +        * characters except full stop and forward slash are replaced with
> > +        * hyphens.
> > +        */
> > +       s = *filename;
> > +       while (*s) {
> > +               c = *s;
> > +               if (isalnum(c))
> > +                       *s = tolower(c);
> > +               else if (c != '.' && c != '/')
> > +                       *s = '-';
> > +               s++;
> > +       }
> > +
> > +       ret = firmware_request_nowarn(firmware, *filename, cs35l56->base.dev);
> > +       if (ret) {
> > +               dev_dbg(cs35l56->base.dev, "Failed to request '%s'\n", *filename);
> > +               kfree(*filename);
> > +               *filename = NULL;
> > +       } else {
> > +               dev_dbg(cs35l56->base.dev, "Found '%s'\n", *filename);
>
> I may be wrong but *filename seems leaked to me on this path. I noticed it
> is could be duplicated on cs_dsp_debugfs_save_wmfwname() but the original
> copy seems left aside.

It's not a leak. The file has been found so the name of the found
file is returned (it's used by other code). But there is a leak of that
memory elsewhere.

> > +       }
> > +
> > +       return ret;
> > +}