Re: [PATCH] mailbox: tegra-hsp: Add check for devm_kstrdup_const()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Mar 01, 2025 at 10:29:29AM -0600, Jassi Brar wrote:
> On Tue, Feb 18, 2025 at 8:28 PM Haoxiang Li <haoxiang_li2024@xxxxxxx> wrote:
> >
> > Add check for the return value of devm_kstrdup_const() in
> > tegra_hsp_doorbell_create() to catch potential exception.
> >
> > Fixes: a54d03ed01b4 ("mailbox: tegra-hsp: use devm_kstrdup_const()")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Haoxiang Li <haoxiang_li2024@xxxxxxx>
> > ---
> >  drivers/mailbox/tegra-hsp.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> > index c1981f091bd1..773a1cf6d93d 100644
> > --- a/drivers/mailbox/tegra-hsp.c
> > +++ b/drivers/mailbox/tegra-hsp.c
> > @@ -285,6 +285,8 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
> >         db->channel.hsp = hsp;
> >
> >         db->name = devm_kstrdup_const(hsp->dev, name, GFP_KERNEL);
> > +       if (!db->name)
> > +               return ERR_PTR(-ENOMEM);
> 
>  tegra_hsp_doorbell.name seems unused, so maybe just get rid of it...  Thierry ?

I think I had at one point used the name in error messages and had ideas
about maybe exposing some information via debugfs. In both cases the
name would've been an easy way to make this more human readable. As it
turns out these errors are very rare (I don't think I've ever seen any)
and I'm not sure there's a need for debugfs.

So yeah, I think one could make a case for removing the name. The amount
of memory it "wastes" is tiny and it's quite probable that it will end
up in some area that would be padding otherwise. Furthermore there's no
way this memory will ever be *not* read-only kernel data, so this is
always going to be a no-op anyway (well, not exactly, but it'll simply
be an assignment of name to db->name, so no actual copying will be done)
and hence this will never actually fail.

So I don't think this patch makes sense. If somebody really wants to
remove the name, that'd be fine with me, but I don't see a strong need.

I suppose not doing anything would increase the chances of a similar
patch being posted again and again over the next few years, so maybe we
should just get rid of it.

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux