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

On Mon, Sep 14, 2026 at 01:32:49PM -0700, Jonathan Cameron wrote:

quoted

+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..

quoted

+	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