Re: [Alsa-devel] [PATCH] Driver core: Don't ignore error returnsfrom probing

[Cornelia Huck]

On Fri, 6 Oct 2006 11:41:05 +0200 (CEST),
Jaroslav Kysela <perex@xxxxxxx> wrote:

> > Hm, I don't think we should call device_release_driver if
> > bus_attach_device failed (and I think calling bus_remove_device if
> > bus_attach_device failed is unintuitive). I did a patch that added a
> > function which undid just the things bus_add_device did (here:
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=115816560424389&w=2),
> > which unfortunately got lost somewhere... (I'll rebase and resend.)
> 
> Yes, but it might be better to check dev->is_registered flag in 
> bus_remove_device() before device_release_driver() call to save some code, 
> rather than reuse most of code in bus_delete_device().

If we undid things (symlinks et al.) in the order we added them, we can
factor out bus_delete_device() from bus_remove_device() and avoid both
code duplication and calling bus_remove_device() if bus_attach_device()
failed. Something like the patch below (untested).


--- linux-2.6-CH.orig/drivers/base/base.h
+++ linux-2.6-CH/drivers/base/base.h
@@ -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);
--- linux-2.6-CH.orig/drivers/base/bus.c
+++ linux-2.6-CH/drivers/base/bus.c
@@ -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)
@@ -424,29 +424,44 @@ int bus_attach_device(struct device * de
 }
 
 /**
- *	bus_remove_device - remove device from bus
- *	@dev:	device to be removed
+ *     bus_delete_device - undo bus_add_device
+ *     @dev:   device being deleted
  *
- *	- Remove symlink from bus's directory.
- *	- Delete device from bus's list.
- *	- Detach from its driver.
- *	- Drop reference taken in bus_add_device().
+ *     - Remove symlink from bus's directory.
+ *     - Remove attributes.
+ *     - Drop reference taken in bus_add_device().
  */
-void bus_remove_device(struct device * dev)
+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);
-		dev->is_registered = 0;
-		klist_del(&dev->knode_bus);
-		pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
-		device_release_driver(dev);
 		put_bus(dev->bus);
 	}
 }
 
+/**
+ *	bus_remove_device - remove device from bus
+ *	@dev:	device to be removed
+ *
+ *	- Remove symlink from bus's directory.
+ *	- Delete device from bus's list.
+ *	- Detach from its driver.
+ *	- Drop reference taken in bus_add_device().
+ */
+void bus_remove_device(struct device * dev)
+{
+	if (!dev->bus)
+		return;
+	dev->is_registered = 0;
+	klist_del(&dev->knode_bus);
+	pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
+	device_release_driver(dev);
+	bus_delete_device(dev);
+}
+
 static int driver_add_attrs(struct bus_type * bus, struct device_driver * drv)
 {
 	int error = 0;
--- linux-2.6-CH.orig/drivers/base/core.c
+++ linux-2.6-CH/drivers/base/core.c
@@ -479,7 +479,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);
 
@@ -498,6 +500,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:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/