sysdev: detecting multiple driver registrations

[Ben Dooks]

I've just found how easy it is to accidentally register
a sysdev_driver for two different classes. When this
happens, bad things happen as the sysdev_driver structure
keeps has the list entry for the driver registration.

The following patch makes a WARN_ON() if this happens,
although I think BUG_ON or returning -EAGAIN could also be valid
responses to this.

Index: linux-2.6.24-quilt5/drivers/base/sys.c
===================================================================
--- linux-2.6.24-quilt5.orig/drivers/base/sys.c
+++ linux-2.6.24-quilt5/drivers/base/sys.c
@@ -169,6 +169,11 @@ int sysdev_driver_register(struct sysdev
 {
 	int err = 0;
 
+	/* Check whether this driver has already been added to a class. */
+
+	WARN_ON(drv->entry.next != drv->entry.prev);
+	WARN_ON(drv->entry.next != NULL);
+
 	mutex_lock(&sysdev_drivers_lock);
 	if (cls && kset_get(&cls->kset)) {
 		list_add_tail(&drv->entry, &cls->drivers);


My first question is, whether people think that this check
is a sane and worthwhile thing to do.

The second question is, should this be BUG_ON, WARN_ON or
return an error (with optional print)?

And my third question is that a number of drivers are
assuming an NULL initialised 'struct list_head' is a
valid setup for an include/linux/list.h list. Is
there a case of using LIST_HEAD_INIT() on all sysdev
driver structures? If not, should we be using
INIT_LIST_HEAD() in sysdev_driver_register(). The header
is not clear on what should be done.

-- 
Ben
--
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/