LKML Archive on
help / color / mirror / Atom feed
* sysdev: detecting multiple driver registrations
@ 2008-02-18 14:51 Ben Dooks
  0 siblings, 0 replies; only message in thread
From: Ben Dooks @ 2008-02-18 14:51 UTC (permalink / raw)
  To: gregkh, linux-kernel

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-> != drv->entry.prev);
+	WARN_ON(drv-> != NULL);
 	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.


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2008-02-18 14:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-18 14:51 sysdev: detecting multiple driver registrations Ben Dooks

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).