Re: bus_add_device() losing an error return from the probe() method

[Andrew Morton <akpm@osdl.org>]

Rene Herman <rene.herman@keyaccess.nl> wrote:
>
>  ===================================================================
>  --- local.orig/drivers/base/bus.c	2006-02-27 19:22:08.000000000 +0100
>  +++ local/drivers/base/bus.c	2006-03-24 04:27:02.000000000 +0100
>  @@ -363,19 +363,21 @@ static void device_remove_attrs(struct b
>   int bus_add_device(struct device * dev)
>   {
>   	struct bus_type * bus = get_bus(dev->bus);
>  -	int error = 0;
>  +	int error;
>   
>   	if (bus) {
>   		pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
>  -		device_attach(dev);
>  +		error = device_attach(dev);
>  +		if (error < 0)
>  +			return error;
>   		klist_add_tail(&dev->knode_bus, &bus->klist_devices);
>   		error = device_add_attrs(bus, dev);
>  -		if (!error) {
>  -			sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
>  -			sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, "bus");
>  -		}
>  +		if (error)
>  +			return error;
>  +		sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
>  +		sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, "bus");
>   	}
>  -	return error;
>  +	return 0;
>   }
>   

Looks sane, but please don't sprinkle `return' statements all over a
function in this manner.


--- devel/drivers/base/bus.c~bus_add_device-losing-an-error-return-from-the-probe-method	2006-03-25 17:46:34.000000000 -0800
+++ devel-akpm/drivers/base/bus.c	2006-03-25 17:49:45.000000000 -0800
@@ -372,14 +372,17 @@ int bus_add_device(struct device * dev)
 
 	if (bus) {
 		pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
-		device_attach(dev);
+		error = device_attach(dev);
+		if (error < 0)
+			goto out;
 		klist_add_tail(&dev->knode_bus, &bus->klist_devices);
 		error = device_add_attrs(bus, dev);
-		if (!error) {
-			sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
-			sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, "bus");
-		}
+		if (error)
+			goto out;
+		sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
+		sysfs_create_link(&dev->kobj,&dev->bus->subsys.kset.kobj,"bus");
 	}
+out:
 	return error;
 }
 

It's a little surprising that this function returns "OK" if bus==NULL.

Note that sysfs_create_link() can fail too.  This was one optimistic
function.