Re: [PATCH v11 1/7] firmware: smccc: Add an Arm SMCCC bus

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Sep 14, 2026 at 01:32:49PM -0700, Jonathan Cameron wrote:
> > +struct arm_smccc_device *arm_smccc_device_register(const char *name, u32 func_id)
> > +{
> > +	int ret;
> > +	struct arm_smccc_device *smccc_dev;
> > +
> > +	if (!name)
> > +		return ERR_PTR(-EINVAL);
> > +
> > +	smccc_dev = kzalloc_obj(*smccc_dev);
> > +	if (!smccc_dev)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	smccc_dev->func_id = func_id;
> > +	smccc_dev->dev.bus = &arm_smccc_bus_type;
> > +	smccc_dev->dev.release = arm_smccc_release_device;
> > +
> > +	ret = dev_set_name(&smccc_dev->dev, "%s", name);
> 
> Does protecting the string defeat the nice underlying const handling?
> e.g.
> 	ret = dev_set_name(&smccc_dev->dev, name);
> might be better.

Pedenatically the %s is better as it doesn't restrict name to not
include % characters. 

> you'd often see this between an device_initialize() and device_add()
> and then we'd be relying on the device_put() to clean it up.
> 
> So as this stands this is fragile as any error paths that later
> get added...

Yes, but as written it is OK, and this is a common pattern in the
kernel. I agree it is fragile tricky..

Still it isn't an urgent reason to change it around, but the best
pattern is to put the allocate, dev.release=, and device_initialize()
in one 'alloc' function. Then the other function calls it and always
unwinds with put_device. Use device_add().

This avoids mixing the different kfree/put_device error unwind regimes
into the same function..

> > +	if (ret) {
> > +		kfree(smccc_dev);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> 
> here can't free the name allocation.

The funky reasoning is since name failed there is no name allocation
to free

And then device_register() failure must always be done with
put_device, so it will capture the name. As long as name/register are
a pair with nothing between it is "fine".

Jason