Index: local/drivers/base/bus.c
===================================================================
--- 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;
}
/**
Rene Herman <[email protected]> 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.
Andrew Morton wrote:
> Looks sane, but please don't sprinkle `return' statements all over a
> function in this manner.
I actually prefer the multiple returns. You then don't have to "visually
scroll down" to the label to see what would happen when reading the
code. Even when there's common code before the return, I've never seen
GCC not optimise that to the goto form itself. You obviously the boss
though.
> 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.
I assume that Greg hasn't commented yet since he's busy rewriting it
all, so that'll be okay :-)
Rene.
Rene Herman <[email protected]> wrote:
> Andrew Morton wrote:
>> Looks sane, but please don't sprinkle `return' statements all over a
>> function in this manner.
>
> I actually prefer the multiple returns. You then don't have to "visually
> scroll down" to the label to see what would happen when reading the
> code. Even when there's common code before the return, I've never seen
> GCC not optimise that to the goto form itself. You obviously the boss
> though.
I tried both, and I found out that having multiple non-trivial points of
return enables you to easily create a lot of bugs.
--
Ich danke GMX daf?r, die Verwendung meiner Adressen mittels per SPF
verbreiteten L?gen zu sabotieren.