[01/12] driver core fixes: make_class_name() retval check

[01/12] driver core fixes: make_class_name() retval check

[Cornelia Huck]

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

make_class_name() may return an error pointer. Audit the callers in the
driver core.

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

 class.c |   11 ++++++++++-
 core.c  |   31 +++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff -Naurp linux-2.6.18-rc6/drivers/base/class.c linux-2.6.18-rc6+CH/drivers/base/class.c
--- linux-2.6.18-rc6/drivers/base/class.c	2006-09-12 14:18:40.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/class.c	2006-09-12 16:17:02.000000000 +0200
@@ -596,6 +596,11 @@ int class_device_add(struct class_device
 	if (class_dev->dev) {
 		class_name = make_class_name(class_dev->class->name,
 					     &class_dev->kobj);
+		if (IS_ERR(class_name)) {
+			error = PTR_ERR(class_name);
+			class_name = NULL;
+			goto out6;
+		}
 		error = sysfs_create_link(&class_dev->kobj,
 					  &class_dev->dev->kobj, "device");
 		if (error)
@@ -736,7 +741,11 @@ void class_device_del(struct class_devic
 		class_name = make_class_name(class_dev->class->name,
 					     &class_dev->kobj);
 		sysfs_remove_link(&class_dev->kobj, "device");
-		sysfs_remove_link(&class_dev->dev->kobj, class_name);
+		if (!IS_ERR(class_name))
+			sysfs_remove_link(&class_dev->dev->kobj, class_name);
+		else
+			/* Hmm, don't know what else to do */
+			class_name = NULL;
 	}
 	sysfs_remove_link(&class_dev->kobj, "subsystem");
 	class_device_remove_file(class_dev, &class_dev->uevent_attr);
diff -Naurp linux-2.6.18-rc6/drivers/base/core.c linux-2.6.18-rc6+CH/drivers/base/core.c
--- linux-2.6.18-rc6/drivers/base/core.c	2006-09-12 14:18:57.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/core.c	2006-09-12 16:17:02.000000000 +0200
@@ -437,7 +437,11 @@ int device_add(struct device *dev)
 		if ((parent) && (!device_is_virtual(dev))) {
 			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 (!IS_ERR(class_name))
+				sysfs_create_link(&dev->parent->kobj,
+						  &dev->kobj, class_name);
+			else
+				class_name = NULL;
 		}
 	}
 
@@ -557,12 +561,16 @@ void device_del(struct device * dev)
 	if (dev->class) {
 		sysfs_remove_link(&dev->kobj, "subsystem");
 		sysfs_remove_link(&dev->class->subsys.kset.kobj, dev->bus_id);
-		class_name = make_class_name(dev->class->name, &dev->kobj);
 		if ((parent) && (!device_is_virtual(dev))) {
+			class_name = make_class_name(dev->class->name,
+						     &dev->kobj);
 			sysfs_remove_link(&dev->kobj, "device");
-			sysfs_remove_link(&dev->parent->kobj, class_name);
+			if (!IS_ERR(class_name)) {
+				sysfs_remove_link(&dev->parent->kobj,
+						  class_name);
+				kfree(class_name);
+			}
 		}
-		kfree(class_name);
 		down(&dev->class->sem);
 		list_del_init(&dev->node);
 		up(&dev->class->sem);
@@ -763,9 +771,14 @@ 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 (IS_ERR(old_class_name)) {
+			error = PTR_ERR(old_class_name);
+			old_class_name = NULL;
+			goto out;
+		}
+	}
 	if (dev->class) {
 		old_symlink_name = kmalloc(BUS_ID_SIZE, GFP_KERNEL);
 		if (!old_symlink_name)
@@ -779,11 +792,12 @@ 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) {
+		if (!IS_ERR(new_class_name)) {
 			sysfs_create_link(&dev->parent->kobj, &dev->kobj,
 					  new_class_name);
 			sysfs_remove_link(&dev->parent->kobj, old_class_name);
-		}
+		} else
+			new_class_name = NULL;
 	}
 	if (dev->class) {
 		sysfs_remove_link(&dev->class->subsys.kset.kobj,
@@ -791,6 +805,7 @@ int device_rename(struct device *dev, ch
 		sysfs_create_link(&dev->class->subsys.kset.kobj, &dev->kobj,
 				  dev->bus_id);
 	}
+out:
 	put_device(dev);
 
 	kfree(old_class_name);

[02/12] driver core fixes: device_register() retval check in platform.c

[Cornelia Huck]

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

Check the return value of device_register() in platform_bus_init().

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

 platform.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

--- linux-2.6.18-rc6/drivers/base/platform.c	2006-09-12 14:12:10.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/platform.c	2006-09-12 16:25:50.000000000 +0200
@@ -563,8 +563,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

[03/12] driver core fixes: fixup platform_device_register_simple()

[Cornelia Huck]

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

Remember to remove allocated resources if platform_device_add() fails.
Introduce a helper function platform_device_del_resources() for this,
which can also be used by platform_device_del().

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

 platform.c |   29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

--- linux-2.6.18-rc6/drivers/base/platform.c	2006-09-12 16:37:21.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/platform.c	2006-09-12 16:39:10.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);

[04/12] driver core fixes: retval check in class_register()

[Cornelia Huck]

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

Check for return value of add_class_attrs() in class_register().

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

 class.c |    2 ++
 1 file changed, 2 insertions(+)

--- linux-2.6.18-rc6/drivers/base/class.c	2006-09-12 16:19:43.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/class.c	2006-09-12 17:01:12.000000000 +0200
@@ -153,6 +153,8 @@ int class_register(struct class * cls)
 	error = subsystem_register(&cls->subsys);
 	if (!error) {
 		error = add_class_attrs(class_get(cls));
+		if (error)
+			subsystem_unregister(&cls->subsys);
 		class_put(cls);
 	}
 	return error;

[05/12] driver core fixes: sysfs_create_link() retval check in class.c

[Cornelia Huck]

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

Check for return value of sysfs_create_link() in class_device_add().

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

 class.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- linux-2.6.18-rc6/drivers/base/class.c	2006-09-12 17:01:56.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/class.c	2006-09-12 17:03:21.000000000 +0200
@@ -562,7 +562,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;

[06/12] driver core fixes: bus_add_attrs() retval check

[Cornelia Huck]

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

Check return value of bus_add_attrs() in bus_register().

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

 bus.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- linux-2.6.18-rc6/drivers/base/bus.c	2006-09-12 14:15:16.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/bus.c	2006-09-12 17:10:50.000000000 +0200
@@ -744,11 +744,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:

[07/12] driver core fixes: bus_add_device() cleanup on error

[Cornelia Huck]

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

Correct cleanup in the error path of bus_add_device().

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

 bus.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

--- linux-2.6.18-rc6/drivers/base/bus.c	2006-09-12 17:18:25.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/bus.c	2006-09-12 17:17:47.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;
 }

[08/12] driver core fixes: device_add() cleanup on error

[Cornelia Huck]

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

Check for return code of device_create_file() and correct cleanup in
the error case in device_add().

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

 core.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

--- linux-2.6.18-rc6/drivers/base/core.c	2006-09-12 16:19:43.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/core.c	2006-09-12 18:27:55.000000000 +0200
@@ -406,14 +406,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;
@@ -423,7 +425,7 @@ int device_add(struct device *dev)
 		error = device_create_file(dev, attr);
 		if (error) {
 			kfree(attr);
-			goto attrError;
+			goto ueventattrError;
 		}
 
 		dev->devt_attr = attr;
@@ -479,6 +481,8 @@ int device_add(struct device *dev)
 		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);

[09/12] driver core fixes: bus_attach_device() retval check

[Cornelia Huck]

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

Check for return value of bus_attach_device() in device_add(). Add a
function bus_delete_device() that undos the effects of bus_add_device().

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

 base.h |    1 +
 bus.c  |   21 ++++++++++++++++++++-
 core.c |    6 +++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

diff -Naurp linux-2.6.18-rc6/drivers/base/base.h linux-2.6.18-rc6+CH/drivers/base/base.h
--- linux-2.6.18-rc6/drivers/base/base.h	2006-09-12 14:15:16.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/base.h	2006-09-12 18:42:08.000000000 +0200
@@ -17,6 +17,7 @@ extern int attribute_container_init(void
 
 extern int bus_add_device(struct device * dev);
 extern int 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-rc6/drivers/base/bus.c linux-2.6.18-rc6+CH/drivers/base/bus.c
--- linux-2.6.18-rc6/drivers/base/bus.c	2006-09-12 18:28:12.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/bus.c	2006-09-12 18:44:24.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)
@@ -420,6 +420,25 @@ int bus_attach_device(struct device * de
 }
 
 /**
+ *     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-rc6/drivers/base/core.c linux-2.6.18-rc6+CH/drivers/base/core.c
--- linux-2.6.18-rc6/drivers/base/core.c	2006-09-12 18:40:36.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/core.c	2006-09-12 18:41:31.000000000 +0200
@@ -456,7 +456,9 @@ int device_add(struct device *dev)
 	if ((error = bus_add_device(dev)))
 		goto BusError;
 	kobject_uevent(&dev->kobj, KOBJ_ADD);
-	bus_attach_device(dev);
+	error = bus_attach_device(dev);
+	if (error)
+		goto attachError;
 	if (parent)
 		klist_add_tail(&dev->knode_parent, &parent->klist_children);
 
@@ -470,6 +472,8 @@ int device_add(struct device *dev)
  	kfree(class_name);
 	put_device(dev);
 	return error;
+ attachError:
+	bus_delete_device(dev);
  BusError:
 	device_pm_remove(dev);
  PMError:

[10/12] driver core fixes: sysfs_create_link() retval check in core.c

[Cornelia Huck]

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

Check for return value of sysfs_create_link() in device_add() and
device_rename(). Add helper functions device_add_class_symlinks() and
device_remove_class_symlinks() to make the code easier to read.

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

 core.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 87 insertions(+), 31 deletions(-)

--- linux-2.6.18-rc6/drivers/base/core.c	2006-09-12 18:50:10.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/core.c	2006-09-13 10:29:33.000000000 +0200
@@ -357,6 +357,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.
@@ -371,7 +432,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);
@@ -431,22 +491,8 @@ int device_add(struct device *dev)
 		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) && (!device_is_virtual(dev))) {
-			sysfs_create_link(&dev->kobj, &dev->parent->kobj, "device");
-			class_name = make_class_name(dev->class->name, &dev->kobj);
-			if (!IS_ERR(class_name))
-				sysfs_create_link(&dev->parent->kobj,
-						  &dev->kobj, class_name);
-			else
-				class_name = NULL;
-		}
-	}
-
+	if ((error = device_add_class_symlinks(dev)))
+		goto SymlinkError;
 	if ((error = device_add_attrs(dev)))
 		goto AttrsError;
 	if ((error = device_add_groups(dev)))
@@ -469,7 +515,6 @@ int device_add(struct device *dev)
 		up(&dev->class->sem);
 	}
  Done:
- 	kfree(class_name);
 	put_device(dev);
 	return error;
  attachError:
@@ -481,6 +526,8 @@ 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);
@@ -770,7 +817,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);
@@ -787,38 +834,47 @@ int device_rename(struct device *dev, ch
 			goto out;
 		}
 	}
-	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);
+	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 (!IS_ERR(new_class_name)) {
-			sysfs_create_link(&dev->parent->kobj, &dev->kobj,
+			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);
 		} else
 			new_class_name = NULL;
 	}
 	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);
+				  old_device_name);
+		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? */
+			dev_err(dev, "%s: sysfs_create_symlink failed (%d)\n",
+				__FUNCTION__, error);
+		}
 	}
 out:
 	put_device(dev);
 
 	kfree(old_class_name);
 	kfree(new_class_name);
-	kfree(old_symlink_name);
+	kfree(old_device_name);
 
 	return error;
 }

[11/12] driver core fixes: device_create_file() retval check in dmapool.c

[Cornelia Huck]

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

Check for device_create_file() return value in dma_pool_create().

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

 dmapool.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

--- linux-2.6.18-rc6/drivers/base/dmapool.c	2006-09-13 10:55:47.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/dmapool.c	2006-09-13 10:55:03.000000000 +0200
@@ -141,11 +141,20 @@ dma_pool_create (const char *name, struc
 	init_waitqueue_head (&retval->waitq);
 
 	if (dev) {
+		int ret;
+
 		down (&pools_lock);
 		if (list_empty (&dev->dma_pools))
-			device_create_file (dev, &dev_attr_pools);
+			ret = device_create_file (dev, &dev_attr_pools);
+		else
+			ret = 0;
 		/* note:  not currently insisting "name" be unique */
-		list_add (&retval->pools, &dev->dma_pools);
+		if (!ret)
+			list_add (&retval->pools, &dev->dma_pools);
+		else {
+			kfree(retval);
+			retval = NULL;
+		}
 		up (&pools_lock);
 	} else
 		INIT_LIST_HEAD (&retval->pools);

[12/12] driver core fixes: sysfs_create_group() retval in topology.c

[Cornelia Huck]

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

Return the return value of sysfs_create_group() in topology_add_dev().

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

 topology.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- linux-2.6.18-rc6/drivers/base/topology.c	2006-09-04 04:19:48.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/topology.c	2006-09-13 10:53:29.000000000 +0200
@@ -97,8 +97,7 @@ static struct attribute_group topology_a
 /* Add/Remove cpu_topology interface for CPU device */
 static int __cpuinit topology_add_dev(struct sys_device * sys_dev)
 {
-	sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
-	return 0;
+	return sysfs_create_group(&sys_dev->kobj, &topology_attr_group);
 }
 
 static int __cpuinit topology_remove_dev(struct sys_device * sys_dev)

Re: [03/12] driver core fixes: fixup platform_device_register_simple()

[Dmitry Torokhov]

Hi,

On Wednesday 13 September 2006 12:38, Cornelia Huck wrote:
> From: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> Remember to remove allocated resources if platform_device_add() fails.
> Introduce a helper function platform_device_del_resources() for this,
> which can also be used by platform_device_del().
> 

platform_device_add() already releases all resources in case of failure.
Memory allocated for resource structures is released by
platform_device_release(). I do not think this patch is needed.

As fas as platform_device_register_somple() goes it should just die and
users should be converted to platofrm_device_alloc/add.

-- 
Dmitry

Re: [03/12] driver core fixes: fixup platform_device_register_simple()

[Cornelia Huck]

On Wed, 13 Sep 2006 22:31:05 -0400,
Dmitry Torokhov <dtor@insightbb.com> wrote:

> platform_device_add() already releases all resources in case of failure.
> Memory allocated for resource structures is released by
> platform_device_release(). I do not think this patch is needed.

Uh, of course, you're right.

Greg, please disregard this patch.

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

Re: [07/12] driver core fixes: bus_add_device() cleanup on error

[Cornelia Huck]

On Wed, 13 Sep 2006 10:27:02 -0700,
Joe Perches <joe@perches.com> wrote:

> better to have a separate error return and a no error return

How about this one?

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

Correct cleanup in the error path of bus_add_device().

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

 bus.c |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

--- linux-2.6.18-rc6/drivers/base/bus.c	2006-09-14 09:32:16.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/bus.c	2006-09-14 09:35:30.000000000 +0200
@@ -372,19 +372,31 @@ 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_create;
+		put_bus(dev->bus);
 	}
-out:
+	return 0;
+
+out_create:
+	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);
 	return error;
 }
 

Re: [10/12] driver core fixes: sysfs_create_link() retval check in core.c

[Cornelia Huck]

On Wed, 13 Sep 2006 10:34:02 -0700,
Joe Perches <joe@perches.com> wrote:

> Please make these styles return 0;

Like this?

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

Check for return value of sysfs_create_link() in device_add() and
device_rename(). Add helper functions device_add_class_symlinks() and
device_remove_class_symlinks() to make the code easier to read.

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

 core.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 87 insertions(+), 31 deletions(-)

--- linux-2.6.18-rc6/drivers/base/core.c	2006-09-12 18:50:10.000000000 +0200
+++ linux-2.6.18-rc6+CH/drivers/base/core.c	2006-09-13 10:29:33.000000000 +0200
@@ -357,6 +357,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 0;
+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.
@@ -371,7 +432,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);
@@ -431,22 +491,8 @@ int device_add(struct device *dev)
 		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) && (!device_is_virtual(dev))) {
-			sysfs_create_link(&dev->kobj, &dev->parent->kobj, "device");
-			class_name = make_class_name(dev->class->name, &dev->kobj);
-			if (!IS_ERR(class_name))
-				sysfs_create_link(&dev->parent->kobj,
-						  &dev->kobj, class_name);
-			else
-				class_name = NULL;
-		}
-	}
-
+	if ((error = device_add_class_symlinks(dev)))
+		goto SymlinkError;
 	if ((error = device_add_attrs(dev)))
 		goto AttrsError;
 	if ((error = device_add_groups(dev)))
@@ -469,7 +515,6 @@ int device_add(struct device *dev)
 		up(&dev->class->sem);
 	}
  Done:
- 	kfree(class_name);
 	put_device(dev);
 	return error;
  attachError:
@@ -481,6 +526,8 @@ 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);
@@ -770,7 +817,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);
@@ -787,38 +834,47 @@ int device_rename(struct device *dev, ch
 			goto out;
 		}
 	}
-	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);
+	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 (!IS_ERR(new_class_name)) {
-			sysfs_create_link(&dev->parent->kobj, &dev->kobj,
+			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);
 		} else
 			new_class_name = NULL;
 	}
 	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);
+				  old_device_name);
+		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? */
+			dev_err(dev, "%s: sysfs_create_symlink failed (%d)\n",
+				__FUNCTION__, error);
+		}
 	}
 out:
 	put_device(dev);
 
 	kfree(old_class_name);
 	kfree(new_class_name);
-	kfree(old_symlink_name);
+	kfree(old_device_name);
 
 	return error;
 }

Re: [01/12] driver core fixes: make_class_name() retval check

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

hoi :)

On Wed, Sep 13, 2006 at 06:38:34PM +0200, Cornelia Huck wrote:
>  	if (class_dev->dev) {
>  		class_name = make_class_name(class_dev->class->name,
>  					     &class_dev->kobj);
> +		if (IS_ERR(class_name)) {
> +			error = PTR_ERR(class_name);
> +			class_name = NULL;
> +			goto out6;
> +		}

perhaps it makes sense to return NULL in make_class_name on error,
to have consistent error return values?

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [01/12] driver core fixes: make_class_name() retval check

[Cornelia Huck]

On Thu, 14 Sep 2006 14:01:33 +0200,
Martin Waitz <tali@admingilde.org> wrote:

> perhaps it makes sense to return NULL in make_class_name on error,
> to have consistent error return values?

It should make the code a bit nicer, I think. But I'm not sure if it's
worth the hassle (config_sysfs_deprecated.patch moves make_class_name()
under CONFIG_SYSFS_DEPRECATED).

Anyway, here is a patch against the latest version of Greg's tree:


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

Make make_class_name() return NULL on error and fixup callers in the
driver core.

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

 class.c |   12 ++++++++----
 core.c  |    7 +++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff -Naurp linux-2.6.18-rc7/drivers/base/class.c linux-2.6.18-rc7+CH/drivers/base/class.c
--- linux-2.6.18-rc7/drivers/base/class.c	2006-09-15 10:38:55.000000000 +0200
+++ linux-2.6.18-rc7+CH/drivers/base/class.c	2006-09-15 12:18:30.000000000 +0200
@@ -360,7 +360,7 @@ char *make_class_name(const char *name, 
 
 	class_name = kmalloc(size, GFP_KERNEL);
 	if (!class_name)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	strcpy(class_name, name);
 	strcat(class_name, ":");
@@ -407,8 +407,11 @@ static int make_deprecated_class_device_
 		return 0;
 
 	class_name = make_class_name(class_dev->class->name, &class_dev->kobj);
-	error = sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj,
-				  class_name);
+	if (!class_name)
+		error = sysfs_create_link(&class_dev->dev->kobj,
+					  &class_dev->kobj, class_name);
+	else
+		error = -ENOMEM;
 	kfree(class_name);
 	return error;
 }
@@ -769,7 +772,8 @@ void class_device_del(struct class_devic
 #ifdef CONFIG_SYSFS_DEPRECATED
 		class_name = make_class_name(class_dev->class->name,
 					     &class_dev->kobj);
-		sysfs_remove_link(&class_dev->dev->kobj, class_name);
+		if (class_name)
+			sysfs_remove_link(&class_dev->dev->kobj, class_name);
 #endif
 		sysfs_remove_link(&class_dev->kobj, "device");
 	}
diff -Naurp linux-2.6.18-rc7/drivers/base/core.c linux-2.6.18-rc7+CH/drivers/base/core.c
--- linux-2.6.18-rc7/drivers/base/core.c	2006-09-15 10:39:03.000000000 +0200
+++ linux-2.6.18-rc7+CH/drivers/base/core.c	2006-09-15 11:01:58.000000000 +0200
@@ -443,7 +443,9 @@ int device_add(struct device *dev)
 		if ((parent) && (!device_is_virtual(dev))) {
 			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 (class_name)
+				sysfs_create_link(&dev->parent->kobj,
+						  &dev->kobj, class_name);
 		}
 #endif
 	}
@@ -574,7 +576,8 @@ void device_del(struct device * dev)
 			char *class_name = NULL;
 
 			class_name = make_class_name(dev->class->name, &dev->kobj);
-			sysfs_remove_link(&dev->parent->kobj, class_name);
+			if (class_name)
+				sysfs_remove_link(&dev->parent->kobj, class_name);
 			kfree(class_name);
 #endif
 			sysfs_remove_link(&dev->kobj, "device");