Skip to contentSkip to conversation

Device class reference counting

PATCH In review Thomas Koeller 2 participants 7 messages Jul 30, 2004 – Aug 10, 2004 Driver core

lore
Share & tools
lore

Thomas Koeller

author
#0 lore raw

PATCH 1/1 Device class reference counting

Hi,

I found a little issue with reference counting for device classes in 2.6.8-rc2. Patch attached. Please cc me on any responses, as I am not subsribed to this list.

tk

drivers/base/class.c +2 -2
@@ -353,8 +353,8 @@
 	struct class_interface * class_intf;
 	int error;
 
-	class_dev = class_device_get(class_dev);
-	if (!class_dev || !strlen(class_dev->class_id))
+	if (!strlen(class_dev->class_id)
+		|| !(class_dev = class_device_get(class_dev)))
 		return -EINVAL;
 
 	parent = class_get(class_dev->class);
Signature · 9 lines
--------------------------------------------------

Thomas Koeller, Software Development
Basler Vision Technologies

thomas dot koeller at baslerweb dot com
http://www.baslerweb.com

==============================
#1 lore raw
Quoted 8 lines · Thomas Koeller

Hi,

I found a little issue with reference counting for device classes in 2.6.8-rc2. Patch attached.

drivers/base/class.c line 358

Quoted 8 lines
drivers/base/class.c +2 -2
@@ -353,8 +353,8 @@
 	struct class_interface * class_intf;
 	int error;
 
-	class_dev = class_device_get(class_dev);
-	if (!class_dev || !strlen(class_dev->class_id))
+	if (!strlen(class_dev->class_id)
+		|| !(class_dev = class_device_get(class_dev)))
 		return -EINVAL;

I don't understand what you are trying to fix here. In fact, if class_dev is NULL, you will now oops.

Hm, I guess if class_dev->class_id is null, we will exit with an extra reference grabbed on the class_dev. Is that what you are trying to fix here?

If so, please rework the patch.

thanks,

greg k-h

author
#2 lore raw
Quoted 25 lines · Greg KH
Quoted 7 lines · Thomas Koeller

Hi,

I found a little issue with reference counting for device classes in 2.6.8-rc2. Patch attached.

drivers/base/class.c

Quoted 9 lines
drivers/base/class.c +3 -2
+0200 @@ -353,8 +353,8 @@
 	struct class_interface * class_intf;
 	int error;
 
-	class_dev = class_device_get(class_dev);
-	if (!class_dev || !strlen(class_dev->class_id))
+	if (!strlen(class_dev->class_id)
+		|| !(class_dev = class_device_get(class_dev)))
 		return -EINVAL;

I don't understand what you are trying to fix here. In fact, if class_dev is NULL, you will now oops.

Hm, I guess if class_dev->class_id is null, we will exit with an extra reference grabbed on the class_dev. Is that what you are trying to fix here?

Quoted 5 lines

If so, please rework the patch.

thanks,

greg k-h

You guessed it - pretty obvious, isn't it? Well, I assumed the case where class_dev is NULL didn't matter, as this could only happen if there is a bug in some other place - seems I was wrong. So how's about this patch:

drivers/base/class.c 2004-08-06 11:06:10.983688216 +0200 renamed from drivers/base/class.c 2004-07-14 16:21:33.000000000 +0200 +7 -2
@@ -349,14 +349,19 @@
 
 int class_device_add(struct class_device *class_dev)
 {
-       struct class * parent;
+       struct class * parent = NULL;
        struct class_interface * class_intf;
        int error;
 
        class_dev = class_device_get(class_dev);
-       if (!class_dev || !strlen(class_dev->class_id))
+       if (!class_dev)
                return -EINVAL;
 
+       if (!strlen(class_dev->class_id)) {
+               error = -EINVAL;
+               goto register_done;
+       }
+
        parent = class_get(class_dev->class);
 
        pr_debug("CLASS: registering class device: ID = '%s'\n",
Signature · 9 lines
--------------------------------------------------

Thomas Koeller, Software Development
Basler Vision Technologies

thomas dot koeller at baslerweb dot com
http://www.baslerweb.com

==============================
author
#3 lore raw

Sorry,

seems the patch got messsed up somehow, so I am
resending it:

drivers/base/class.c 2004-08-06 11:06:10.983688216 +0200 renamed from drivers/base/class.c 2004-07-14 16:21:33.000000000 +0200 +7 -2
@@ -349,14 +349,19 @@
 
 int class_device_add(struct class_device *class_dev)
 {
-       struct class * parent;
+       struct class * parent = NULL;
        struct class_interface * class_intf;
        int error;
 
        class_dev = class_device_get(class_dev);
-       if (!class_dev || !strlen(class_dev->class_id))
+       if (!class_dev)
                return -EINVAL;
 
+       if (!strlen(class_dev->class_id)) {
+               error = -EINVAL;
+               goto register_done;
+       }
+
        parent = class_get(class_dev->class);
 
        pr_debug("CLASS: registering class device: ID = '%s'\n",
Signature · 9 lines
--------------------------------------------------

Thomas Koeller, Software Development
Basler Vision Technologies

thomas dot koeller at baslerweb dot com
http://www.baslerweb.com

==============================
#4 lore raw
Quoted 4 lines · Thomas Koeller

Sorry,

seems the patch got messsed up somehow, so I am
resending it:

This patch looks good. But it has the tabs stripped out of it, and no
Signed-off-by: line. Care to resend it with that fixed up?

thanks,

greg k-h

author
#5 lore raw
Quoted 6 lines · Greg KH
Quoted 4 lines · Thomas Koeller

Sorry,

seems the patch got messsed up somehow, so I am
resending it:

Quoted 6 lines

This patch looks good. But it has the tabs stripped out of it, and no
Signed-off-by: line. Care to resend it with that fixed up?

thanks,

greg k-h

Hi greg,

here's the patch again, re-created against 2.6.8-rc3. This time I even managed to read Documentation/SubmittingPatches ;-)

The tabs are all there, if they are stripped again somewhere along the way, it is beyond my control. If this should happen, could you fix that on your side?

thanks,
Thomas

Signed-off-by Thomas Koeller <[email protected]>
drivers/base/class.c +8 -3
@@ -349,15 +349,20 @@ void class_device_initialize(struct clas
 
 int class_device_add(struct class_device *class_dev)
 {
-	struct class * parent;
+	struct class * parent = NULL;
 	struct class_interface * class_intf;
 	int error;
 
 	class_dev = class_device_get(class_dev);
-	if (!class_dev || !strlen(class_dev->class_id))
+	if (!class_dev)
 		return -EINVAL;
 
-	parent = class_get(class_dev->class);
+	if (!strlen(class_dev->class_id)) {
+		error = -EINVAL;
+		goto register_done;
+	}
+
+ 	parent = class_get(class_dev->class);
 
 	pr_debug("CLASS: registering class device: ID = '%s'\n",
 		 class_dev->class_id);
Signature · 9 lines
--------------------------------------------------

Thomas Koeller, Software Development
Basler Vision Technologies

thomas dot koeller at baslerweb dot com
http://www.baslerweb.com

==============================
#6 lore raw
Quoted 16 lines · Thomas Koeller
Quoted 6 lines · Greg KH
Quoted 4 lines · Thomas Koeller

Sorry,

seems the patch got messsed up somehow, so I am
resending it:

Quoted 6 lines

This patch looks good. But it has the tabs stripped out of it, and no
Signed-off-by: line. Care to resend it with that fixed up?

thanks,

greg k-h

Hi greg,

Quoted 7 lines

here's the patch again, re-created against 2.6.8-rc3. This time I even managed to read Documentation/SubmittingPatches ;-)

The tabs are all there, if they are stripped again somewhere along the way, it is beyond my control. If this should happen, could you fix that on your side?

Looks good, I've applied this, thanks.

greg k-h