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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
