[Patch] [mm] Fix bus_rescan_devices() in -mm

[Patch] [mm] Fix bus_rescan_devices() in -mm

[Cornelia Huck]

With the __must_check changes in the driver core,
bus_rescan_devices_helper() now returns the return code of its call to
device_attach(). device_attach() will return < 0 on error, 0 on no
match and 1 on match. This means that bus_rescan_devices() will stop
after the first successful match for a device, which is probably not
what we want. Stopping on error makes sense, however.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

 bus.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/bus.c linux-2.6.18-rc1-mm2+CH/drivers/base/bus.c
--- linux-2.6.18-rc1-mm2/drivers/base/bus.c	2006-07-17 18:15:37.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/bus.c	2006-07-17 18:17:51.000000000 +0200
@@ -572,7 +572,7 @@ static int __must_check bus_rescan_devic
 		if (dev->parent)
 			up(&dev->parent->sem);
 	}
-	return ret;
+	return ret < 0 ? ret : 0;
 }
 
 /**

[Patch] [mm] More driver core fixes for -mm

[Cornelia Huck]

I've looked some more into the __must_check stuff in the driver core,
and tried to fix some functions (especially device_add() is a bit of a
beast; I split off helper functions).

Question: What is considered "good style" concerning symlinks? I would
think I should remove symlinks I created, but most places don't seem to
do this...

-- 
Cornelia Huck
Linux for zSeries Developer
Tel.: +49-7031-16-4837, Mail: cornelia.huck@de.ibm.com

From: Cornelia Huck <cornelia.huck@de.ibm.com>

Fix missing checks of return codes for driver model functions called in
the driver core.

Also fix bus_attach_device(), which didn't take into account that
device_attach() may return 0 or 1 on success.

CC: Greg K-H <greg@kroah.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

 base.h     |    1 
 bus.c      |   24 +++++++++++-
 class.c    |   12 ++++--
 core.c     |  115 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
 platform.c |   11 ++++-
5 files changed, 133 insertions(+), 30 deletions(-)

diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/base.h linux-2.6.18-rc1-mm2+CH/drivers/base/base.h
--- linux-2.6.18-rc1-mm2/drivers/base/base.h	2006-07-17 17:58:13.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/base.h	2006-07-21 13:36:09.000000000 +0200
@@ -17,6 +17,7 @@ extern int attribute_container_init(void
 
 extern int bus_add_device(struct device * dev);
 extern int __must_check bus_attach_device(struct device * dev);
+extern void bus_delete_device(struct device * dev);
 extern void bus_remove_device(struct device * dev);
 extern struct bus_type *get_bus(struct bus_type * bus);
 extern void put_bus(struct bus_type * bus);
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/bus.c linux-2.6.18-rc1-mm2+CH/drivers/base/bus.c
--- linux-2.6.18-rc1-mm2/drivers/base/bus.c	2006-07-21 13:49:57.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/bus.c	2006-07-21 14:51:20.000000000 +0200
@@ -360,7 +360,7 @@ static void device_remove_attrs(struct b
  *	bus_add_device - add device to bus
  *	@dev:	device being added
  *
- *	- Add the device to its bus's list of devices.
+ *	- Add attributes.
  *	- Create link to device's bus.
  */
 int bus_add_device(struct device * dev)
@@ -401,13 +401,33 @@ int bus_attach_device(struct device * de
 
 	if (bus) {
 		ret = device_attach(dev);
-		if (ret == 0)
+		if (ret >= 0)
 			klist_add_tail(&dev->knode_bus, &bus->klist_devices);
 	}
 	return ret;
 }
 
 /**
+ *	bus_delete_device - undo bus_add_device
+ *	@dev:	device being deleted
+ *
+ *	- Remove symlink from bus's directory.
+ *	- Remove attributes.
+ *	- Drop reference taken in bus_add_device().
+ */
+void bus_delete_device(struct device * dev)
+{
+	if (dev->bus) {
+		sysfs_remove_link(&dev->kobj, "subsystem");
+		sysfs_remove_link(&dev->kobj, "bus");
+		sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
+		device_remove_attrs(dev->bus, dev);
+		put_bus(dev->bus);
+	}
+}
+
+
+/**
  *	bus_remove_device - remove device from bus
  *	@dev:	device to be removed
  *
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/class.c linux-2.6.18-rc1-mm2+CH/drivers/base/class.c
--- linux-2.6.18-rc1-mm2/drivers/base/class.c	2006-07-06 06:09:49.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/class.c	2006-07-21 13:43:03.000000000 +0200
@@ -560,7 +560,10 @@ int class_device_add(struct class_device
 		goto out2;
 
 	/* add the needed attributes to this device */
-	sysfs_create_link(&class_dev->kobj, &parent_class->subsys.kset.kobj, "subsystem");
+	error = sysfs_create_link(&class_dev->kobj,
+				  &parent_class->subsys.kset.kobj, "subsystem");
+	if (error)
+		goto out3;
 	class_dev->uevent_attr.attr.name = "uevent";
 	class_dev->uevent_attr.attr.mode = S_IWUSR;
 	class_dev->uevent_attr.attr.owner = parent_class->owner;
@@ -809,10 +812,13 @@ int class_device_rename(struct class_dev
 	if (class_dev->dev) {
 		new_class_name = make_class_name(class_dev->class->name,
 						 &class_dev->kobj);
-		sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
-				  new_class_name);
+		error = sysfs_create_link(&class_dev->dev->kobj,
+					  &class_dev->kobj, new_class_name);
+		if (error)
+			goto out;
 		sysfs_remove_link(&class_dev->dev->kobj, old_class_name);
 	}
+out:
 	class_device_put(class_dev);
 
 	kfree(old_class_name);
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/core.c linux-2.6.18-rc1-mm2+CH/drivers/base/core.c
--- linux-2.6.18-rc1-mm2/drivers/base/core.c	2006-07-17 17:58:13.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/core.c	2006-07-21 14:51:00.000000000 +0200
@@ -353,6 +353,67 @@ void device_initialize(struct device *de
 	device_init_wakeup(dev, 0);
 }
 
+static int device_add_class_symlinks(struct device *dev)
+{
+	int error;
+	char *class_name;
+
+	if (!dev->class)
+		return 0;
+	error = sysfs_create_link(&dev->kobj, &dev->class->subsys.kset.kobj,
+				  "subsystem");
+	if (error)
+		goto out;
+	error = sysfs_create_link(&dev->class->subsys.kset.kobj, &dev->kobj,
+				  dev->bus_id);
+	if (error)
+		goto out_subsys;
+	if (dev->parent) {
+		error = sysfs_create_link(&dev->kobj, &dev->parent->kobj,
+					  "device");
+		if (error)
+			goto out_busid;
+		class_name = make_class_name(dev->class->name, &dev->kobj);
+		if (IS_ERR(class_name)) {
+			error = PTR_ERR(class_name);
+			goto out_busid;
+		}
+		error = sysfs_create_link(&dev->parent->kobj, &dev->kobj,
+					  class_name);
+		kfree(class_name);
+		if (error)
+			goto out_device;
+	}
+	return error;
+out_device:
+	if (dev->parent)
+		sysfs_remove_link(&dev->kobj, "device");
+out_busid:
+	sysfs_remove_link(&dev->class->subsys.kset.kobj, dev->bus_id);
+out_subsys:
+	sysfs_remove_link(&dev->kobj, "subsystem");
+out:
+	return error;
+}
+
+static void device_remove_class_symlinks(struct device *dev)
+{
+	char *class_name = NULL;
+
+	if (!dev->class)
+		return;
+	if (dev->parent) {
+		class_name = make_class_name(dev->class->name, &dev->kobj);
+		if (!IS_ERR(class_name)) {
+			sysfs_remove_link(&dev->parent->kobj, class_name);
+			kfree(class_name);
+		}
+		sysfs_remove_link(&dev->kobj, "device");
+	}
+	sysfs_remove_link(&dev->class->subsys.kset.kobj, dev->bus_id);
+	sysfs_remove_link(&dev->kobj, "subsystem");
+}
+
 /**
  *	device_add - add device to device hierarchy.
  *	@dev:	device.
@@ -367,7 +428,6 @@ void device_initialize(struct device *de
 int device_add(struct device *dev)
 {
 	struct device *parent = NULL;
-	char *class_name = NULL;
 	int error = -EINVAL;
 
 	dev = get_device(dev);
@@ -395,14 +455,16 @@ int device_add(struct device *dev)
 	if (dev->driver)
 		dev->uevent_attr.attr.owner = dev->driver->owner;
 	dev->uevent_attr.store = store_uevent;
-	device_create_file(dev, &dev->uevent_attr);
+	error = device_create_file(dev, &dev->uevent_attr);
+	if (error)
+		goto attrError;
 
 	if (MAJOR(dev->devt)) {
 		struct device_attribute *attr;
 		attr = kzalloc(sizeof(*attr), GFP_KERNEL);
 		if (!attr) {
 			error = -ENOMEM;
-			goto PMError;
+			goto ueventattrError;
 		}
 		attr->attr.name = "dev";
 		attr->attr.mode = S_IRUGO;
@@ -412,24 +474,13 @@ int device_add(struct device *dev)
 		error = device_create_file(dev, attr);
 		if (error) {
 			kfree(attr);
-			goto attrError;
+			goto ueventattrError;
 		}
-
 		dev->devt_attr = attr;
 	}
 
-	if (dev->class) {
-		sysfs_create_link(&dev->kobj, &dev->class->subsys.kset.kobj,
-				  "subsystem");
-		sysfs_create_link(&dev->class->subsys.kset.kobj, &dev->kobj,
-				  dev->bus_id);
-		if (parent) {
-			sysfs_create_link(&dev->kobj, &dev->parent->kobj, "device");
-			class_name = make_class_name(dev->class->name, &dev->kobj);
-			sysfs_create_link(&dev->parent->kobj, &dev->kobj, class_name);
-		}
-	}
-
+	if ((error = device_add_class_symlinks(dev)))
+		goto SymlinkError;
 	if ((error = device_add_attrs(dev)))
 		goto AttrsError;
 	if ((error = device_add_groups(dev)))
@@ -439,7 +490,12 @@ int device_add(struct device *dev)
 	if ((error = bus_add_device(dev)))
 		goto BusError;
 	kobject_uevent(&dev->kobj, KOBJ_ADD);
-	bus_attach_device(dev);
+	if ((error = bus_attach_device(dev))) {
+		if (error < 0)
+			goto attachError;
+		else
+			error = 0;
+	}
 	if (parent)
 		klist_add_tail(&dev->knode_parent, &parent->klist_children);
 
@@ -450,9 +506,10 @@ int device_add(struct device *dev)
 		up(&dev->class->sem);
 	}
  Done:
- 	kfree(class_name);
 	put_device(dev);
 	return error;
+ attachError:
+	bus_delete_device(dev);
  BusError:
 	device_pm_remove(dev);
  PMError:
@@ -460,10 +517,14 @@ int device_add(struct device *dev)
  GroupError:
  	device_remove_attrs(dev);
  AttrsError:
+	device_remove_class_symlinks(dev);
+ SymlinkError:
 	if (dev->devt_attr) {
 		device_remove_file(dev, dev->devt_attr);
 		kfree(dev->devt_attr);
 	}
+ ueventattrError:
+	device_remove_file(dev, &dev->uevent_attr);
  attrError:
 	kobject_uevent(&dev->kobj, KOBJ_REMOVE);
 	kobject_del(&dev->kobj);
@@ -769,17 +830,25 @@ int device_rename(struct device *dev, ch
 	if (old_class_name) {
 		new_class_name = make_class_name(dev->class->name, &dev->kobj);
 		if (new_class_name) {
-			sysfs_create_link(&dev->parent->kobj, &dev->kobj,
-					  new_class_name);
+			error = sysfs_create_link(&dev->parent->kobj,
+						  &dev->kobj, new_class_name);
+			if (error)
+				goto out;
 			sysfs_remove_link(&dev->parent->kobj, old_class_name);
 		}
 	}
 	if (dev->class) {
 		sysfs_remove_link(&dev->class->subsys.kset.kobj,
 				  old_symlink_name);
-		sysfs_create_link(&dev->class->subsys.kset.kobj, &dev->kobj,
-				  dev->bus_id);
+		error = sysfs_create_link(&dev->class->subsys.kset.kobj,
+					  &dev->kobj, dev->bus_id);
+		if (error) {
+			/* Uh... how to unravel this if restoring can fail? */
+			printk(KERN_ERR"%s: sysfs_create_symlink(%s) failed\n",
+			       __FUNCTION__, dev->bus_id);
+		}
 	}
+out:
 	put_device(dev);
 
 	kfree(old_class_name);
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/platform.c linux-2.6.18-rc1-mm2+CH/drivers/base/platform.c
--- linux-2.6.18-rc1-mm2/drivers/base/platform.c	2006-07-06 06:09:49.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/platform.c	2006-07-21 14:04:45.000000000 +0200
@@ -537,8 +537,15 @@ EXPORT_SYMBOL_GPL(platform_bus_type);
 
 int __init platform_bus_init(void)
 {
-	device_register(&platform_bus);
-	return bus_register(&platform_bus_type);
+	int error;
+
+	error = device_register(&platform_bus);
+	if (error)
+		return error;
+	error = bus_register(&platform_bus_type);
+	if (error)
+		device_unregister(&platform_bus);
+	return error;
 }
 
 #ifndef ARCH_HAS_DMA_GET_REQUIRED_MASK

[Patch] [mm] Yet further driver core fixes for -mm

[Cornelia Huck]

This patch goes on top of the previous one and attempts to fix some
more problems in the driver core. These should contain most of the
low-hanging fruit for now...

These patches probably could use some testing by someone else but me :)

-- 
Cornelia Huck
Linux for zSeries Developer
Tel.: +49-7031-16-4837, Mail: cornelia.huck@de.ibm.com

From: Cornelia Huck <cornelia.huck@de.ibm.com>

Some more fixes in the driver core:

- make_class_name() returns an ERR_PTR on error, not NULL.
- fixup some more unwinding on errors.

CC: Greg K-H <greg@kroah.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>

 bus.c      |   24 +++++++++++++++++++-----
 class.c    |    5 ++++-
 core.c     |   46 ++++++++++++++++++++++++++++------------------
 platform.c |   29 ++++++++++++++++++-----------
 4 files changed, 69 insertions(+), 35 deletions(-)

diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/bus.c linux-2.6.18-rc1-mm2+CH/drivers/base/bus.c
--- linux-2.6.18-rc1-mm2/drivers/base/bus.c	2006-07-24 12:59:29.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/bus.c	2006-07-24 18:31:36.000000000 +0200
@@ -372,18 +372,28 @@ int bus_add_device(struct device * dev)
 		pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
 		error = device_add_attrs(bus, dev);
 		if (error)
-			goto out;
+			goto out_put;
 		error = sysfs_create_link(&bus->devices.kobj,
 						&dev->kobj, dev->bus_id);
 		if (error)
-			goto out;
+			goto out_id;
 		error = sysfs_create_link(&dev->kobj,
 				&dev->bus->subsys.kset.kobj, "subsystem");
 		if (error)
-			goto out;
+			goto out_subsys;
 		error = sysfs_create_link(&dev->kobj,
 				&dev->bus->subsys.kset.kobj, "bus");
-	}
+		if (!error)
+			goto out;
+	} else
+		goto out;
+	sysfs_remove_link(&dev->kobj, "subsystem");
+out_subsys:
+	sysfs_remove_link(&bus->devices.kobj, dev->bus_id);
+out_id:
+	device_remove_attrs(bus, dev);
+out_put:
+	put_bus(dev->bus);
 out:
 	return error;
 }
@@ -760,11 +770,15 @@ int bus_register(struct bus_type * bus)
 
 	klist_init(&bus->klist_devices, klist_devices_get, klist_devices_put);
 	klist_init(&bus->klist_drivers, klist_drivers_get, klist_drivers_put);
-	bus_add_attrs(bus);
+	retval = bus_add_attrs(bus);
+	if (retval)
+		goto bus_attrs_fail;
 
 	pr_debug("bus type '%s' registered\n", bus->name);
 	return 0;
 
+bus_attrs_fail:
+	kset_unregister(&bus->drivers);
 bus_drivers_fail:
 	kset_unregister(&bus->devices);
 bus_devices_fail:
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/class.c linux-2.6.18-rc1-mm2+CH/drivers/base/class.c
--- linux-2.6.18-rc1-mm2/drivers/base/class.c	2006-07-24 12:59:29.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/class.c	2006-07-24 15:58:39.000000000 +0200
@@ -153,7 +153,10 @@ int class_register(struct class * cls)
 	error = subsystem_register(&cls->subsys);
 	if (!error) {
 		error = add_class_attrs(class_get(cls));
-		class_put(cls);
+		if (error)
+			subsystem_unregister(&cls->subsys);
+		else
+			class_put(cls);
 	}
 	return error;
 }
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/core.c linux-2.6.18-rc1-mm2+CH/drivers/base/core.c
--- linux-2.6.18-rc1-mm2/drivers/base/core.c	2006-07-24 12:59:29.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/core.c	2006-07-24 18:24:42.000000000 +0200
@@ -804,7 +804,7 @@ int device_rename(struct device *dev, ch
 {
 	char *old_class_name = NULL;
 	char *new_class_name = NULL;
-	char *old_symlink_name = NULL;
+	char *old_device_name = NULL;
 	int error;
 
 	dev = get_device(dev);
@@ -813,33 +813,43 @@ int device_rename(struct device *dev, ch
 
 	pr_debug("DEVICE: renaming '%s' to '%s'\n", dev->bus_id, new_name);
 
-	if ((dev->class) && (dev->parent))
+	if ((dev->class) && (dev->parent)) {
 		old_class_name = make_class_name(dev->class->name, &dev->kobj);
-
-	if (dev->class) {
-		old_symlink_name = kmalloc(BUS_ID_SIZE, GFP_KERNEL);
-		if (!old_symlink_name)
-			return -ENOMEM;
-		strlcpy(old_symlink_name, dev->bus_id, BUS_ID_SIZE);
+		if (IS_ERR(old_class_name)) {
+			error = PTR_ERR(old_class_name);
+			old_class_name = NULL;
+			goto out;
+		}
 	}
-
+	old_device_name = kmalloc(BUS_ID_SIZE, GFP_KERNEL);
+	if (!old_device_name) {
+		error = -ENOMEM;
+		goto out;
+	}
+	strlcpy(old_device_name, dev->bus_id, BUS_ID_SIZE);
 	strlcpy(dev->bus_id, new_name, BUS_ID_SIZE);
 
 	error = kobject_rename(&dev->kobj, new_name);
-
+	if (error) {
+		strlcpy(dev->bus_id, old_device_name, BUS_ID_SIZE);
+		goto out;
+	}
 	if (old_class_name) {
 		new_class_name = make_class_name(dev->class->name, &dev->kobj);
-		if (new_class_name) {
-			error = sysfs_create_link(&dev->parent->kobj,
-						  &dev->kobj, new_class_name);
-			if (error)
-				goto out;
-			sysfs_remove_link(&dev->parent->kobj, old_class_name);
+		if (IS_ERR(new_class_name)) {
+			error = PTR_ERR(new_class_name);
+			new_class_name = NULL;
+			goto out;
 		}
+		error = sysfs_create_link(&dev->parent->kobj,
+					  &dev->kobj, new_class_name);
+		if (error)
+			goto out;
+		sysfs_remove_link(&dev->parent->kobj, old_class_name);
 	}
 	if (dev->class) {
 		sysfs_remove_link(&dev->class->subsys.kset.kobj,
-				  old_symlink_name);
+				  old_device_name);
 		error = sysfs_create_link(&dev->class->subsys.kset.kobj,
 					  &dev->kobj, dev->bus_id);
 		if (error) {
@@ -853,7 +863,7 @@ out:
 
 	kfree(old_class_name);
 	kfree(new_class_name);
-	kfree(old_symlink_name);
+	kfree(old_device_name);
 
 	return error;
 }
diff -Naurp linux-2.6.18-rc1-mm2/drivers/base/platform.c linux-2.6.18-rc1-mm2+CH/drivers/base/platform.c
--- linux-2.6.18-rc1-mm2/drivers/base/platform.c	2006-07-24 12:59:29.000000000 +0200
+++ linux-2.6.18-rc1-mm2+CH/drivers/base/platform.c	2006-07-24 15:39:07.000000000 +0200
@@ -202,6 +202,17 @@ int platform_device_add_resources(struct
 }
 EXPORT_SYMBOL_GPL(platform_device_add_resources);
 
+static void platform_device_del_resources(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < pdev->num_resources; i++) {
+		struct resource *r = &pdev->resource[i];
+		if (r->flags & (IORESOURCE_MEM|IORESOURCE_IO))
+			release_resource(r);
+	}
+}
+
 /**
  *	platform_device_add_data
  *	@pdev:	platform device allocated by platform_device_alloc to add resources to
@@ -296,15 +307,8 @@ EXPORT_SYMBOL_GPL(platform_device_add);
  */
 void platform_device_del(struct platform_device *pdev)
 {
-	int i;
-
 	if (pdev) {
-		for (i = 0; i < pdev->num_resources; i++) {
-			struct resource *r = &pdev->resource[i];
-			if (r->flags & (IORESOURCE_MEM|IORESOURCE_IO))
-				release_resource(r);
-		}
-
+		platform_device_del_resources(pdev);
 		device_del(&pdev->dev);
 	}
 }
@@ -365,17 +369,20 @@ struct platform_device *platform_device_
 	if (num) {
 		retval = platform_device_add_resources(pdev, res, num);
 		if (retval)
-			goto error;
+			goto error_put;
 	}
 
 	retval = platform_device_add(pdev);
 	if (retval)
-		goto error;
+		goto error_resources;
 
 	return pdev;
 
-error:
+error_resources:
+	platform_device_del_resources(pdev);
+error_put:
 	platform_device_put(pdev);
+error:
 	return ERR_PTR(retval);
 }
 EXPORT_SYMBOL_GPL(platform_device_register_simple);

Re: [Patch] [mm] More driver core fixes for -mm

[Andrew Morton]

On Fri, 21 Jul 2006 15:20:00 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> 
> I've looked some more into the __must_check stuff in the driver core,
> and tried to fix some functions (especially device_add() is a bit of a
> beast; I split off helper functions).

OK.

> Question: What is considered "good style" concerning symlinks? I would
> think I should remove symlinks I created, but most places don't seem to
> do this...

Removing symlinks seems like a good idea.  Leaving them around might cause
a subsequent driver load to fail due to EEXIST (assuming that the caller
checks error codes, as if).

I assume you're referring to error paths here?

> -- 
> Cornelia Huck
> Linux for zSeries Developer
> Tel.: +49-7031-16-4837, Mail: cornelia.huck@de.ibm.com
> 
> From: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> Fix missing checks of return codes for driver model functions called in
> the driver core.
> 
> Also fix bus_attach_device(), which didn't take into account that
> device_attach() may return 0 or 1 on success.
> 

Yes, this was nasty (oopses).

> @@ -401,13 +401,33 @@ int bus_attach_device(struct device * de
>  
>  	if (bus) {
>  		ret = device_attach(dev);
> -		if (ret == 0)
> +		if (ret >= 0)
>  			klist_add_tail(&dev->knode_bus, &bus->klist_devices);
>  	}

But I made bus_attach_device() convert the positive return value to zero. 
See
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc1/2.6.18-rc1-mm2/hot-fixes/drivers-base-check-errors-fix.patch.

Is there a reason to propagate this irritating "1" back out of
bus_attach_device() as well?

Re: [Patch] [mm] More driver core fixes for -mm

[Cornelia Huck]

On Tue, 25 Jul 2006 01:08:52 -0700,
Andrew Morton <akpm@osdl.org> wrote:

> Removing symlinks seems like a good idea.  Leaving them around might cause
> a subsequent driver load to fail due to EEXIST (assuming that the caller
> checks error codes, as if).
> 
> I assume you're referring to error paths here?

Yes, that was my reasoning.

> But I made bus_attach_device() convert the positive return value to zero. 
> See
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc1/2.6.18-rc1-mm2/hot-fixes/drivers-base-check-errors-fix.patch.

Missed that, sorry.

> 
> Is there a reason to propagate this irritating "1" back out of
> bus_attach_device() as well?

Probably not. Nobody cares whether the device was bound to a driver or
not.

-- 
Cornelia Huck
Linux for zSeries Developer
Tel.: +49-7031-16-4837, Mail: cornelia.huck@de.ibm.com