Skip to contentSkip to conversation

Fix error path when adding sysfs attributes

PATCH In review Martin Hicks 3 participants 6 messages Jan 16, 2004 SCSI

lore
Share & tools
lore

Martin Hicks

author
#0 lore raw

PATCH 1/1 Fix error path when adding sysfs attributes

Hi James,

Here is a patch that fixes an error path while adding sysfs attributes in drivers/scsi/scsi_sysfs.c

If we encounter an error while registering the sysfs attributes we mark the sdev as dead and unregister it from sysfs. Then we turn around and continue to add new attributes.

Thanks to Christoph and Martin for pointing this out.

thanks
mh

Signature · 2 lines
Martin Hicks                Wild Open Source Inc.
[email protected]     613-266-2296

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.1521 -> 1.1522
# drivers/scsi/scsi_sysfs.c 1.38 -> 1.39
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/01/16 [email protected] 1.1522
# Stop adding sysfs attributes after we call scsi_remove_device()

# when we encounter an error.
# --------------------------------------------
#
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c	Fri Jan 16 13:45:02 2004
+++ b/drivers/scsi/scsi_sysfs.c	Fri Jan 16 13:45:02 2004
@@ -370,8 +370,10 @@
 		for (i = 0; sdev->host->hostt->sdev_attrs[i]; i++) {
 			error = attr_add(&sdev->sdev_gendev,
 					sdev->host->hostt->sdev_attrs[i]);
-			if (error)
+			if (error) {
 				scsi_remove_device(sdev);
+				goto out;
+			}
 		}
 	}

@@ -380,11 +382,14 @@
scsi_sysfs_sdev_attrs[i])) {
error = device_create_file(&sdev->sdev_gendev,
scsi_sysfs_sdev_attrs[i]);
- if (error)
+ if (error) {
scsi_remove_device(sdev);
+ goto out;
+ }
}
}

+ out:
return error;

clean_device:

#1 lore raw
Quoted 5 lines · Martin Hicks
(quoted diff) +0 -0
@@ -380,11 +382,14 @@
 					scsi_sysfs_sdev_attrs[i])) {
 			error = device_create_file(&sdev->sdev_gendev,

Review line 392

Quoted 10 lines
(quoted diff) +4 -1
 					scsi_sysfs_sdev_attrs[i]);
-			if (error)
+			if (error) {
 				scsi_remove_device(sdev);
+				goto out;
+			}
 		}
 	}
 
+	out:

This is tabbed stupidly. Sorry about that.

mh

Signature · 1 line
Martin Hicks || [email protected] || PGP/GnuPG: 0x4C7F2BEE
author
#3 lore raw
Quoted 3 lines · Mike Anderson

While we are cleaning there appears to also be an extra return on line 364 after a goto. It might be good to include this change in the patch.

Good idea. Here's an updated patch that fixes the original bugs, the extra "return" and a bit of whitespace.

thanks
mh

Signature · 2 lines
Martin Hicks                Wild Open Source Inc.
[email protected]     613-266-2296
#4 lore raw
Quoted 10 lines · Martin Hicks
Quoted 3 lines · Mike Anderson

While we are cleaning there appears to also be an extra return on line 364 after a goto. It might be good to include this change in the patch.

Good idea. Here's an updated patch that fixes the original bugs, the extra "return" and a bit of whitespace.

And now the patch...

mh

Signature · 1 line
Martin Hicks || [email protected] || PGP/GnuPG: 0x4C7F2BEE

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.1521 -> 1.1522
# drivers/scsi/scsi_sysfs.c 1.38 -> 1.39
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/01/16 [email protected] 1.1522
# Stop adding sysfs attributes after we call scsi_remove_device()

# when we encounter an error.  Also a small whitespace cleanup
# and removing a useless "return".
# --------------------------------------------
#
diff -Nru a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
--- a/drivers/scsi/scsi_sysfs.c	Fri Jan 16 15:24:41 2004
+++ b/drivers/scsi/scsi_sysfs.c	Fri Jan 16 15:24:41 2004
@@ -361,7 +361,6 @@
 	if (error) {
 		printk(KERN_INFO "error 2\n");
 		goto clean_device;
-		return error;
 	}

get_device(&sdev->sdev_gendev);
@@ -370,8 +369,10 @@
for (i = 0; sdev->host->hostt->sdev_attrs[i]; i++) {
error = attr_add(&sdev->sdev_gendev,
sdev->host->hostt->sdev_attrs[i]);
- if (error)
+ if (error) {
scsi_remove_device(sdev);
+ goto out;
+ }
}
}

@@ -380,11 +381,14 @@
scsi_sysfs_sdev_attrs[i])) {
error = device_create_file(&sdev->sdev_gendev,
scsi_sysfs_sdev_attrs[i]);
- if (error)
+ if (error) {
scsi_remove_device(sdev);
+ goto out;
+ }
}
}

+ out:
return error;

clean_device:
@@ -394,7 +398,6 @@
put_device(&sdev->sdev_gendev);

return error;
-
}

/**

#5 lore raw
Quoted 5 lines · Martin Hicks [[email protected]] wrote:
Quoted 2 lines · Martin Hicks
Quoted 8 lines
Quoted 8 lines · Mike Anderson
Quoted 3 lines

While we are cleaning there appears to also be an extra return on line 364 after a goto. It might be good to include this change in the patch.

Good idea. Here's an updated patch that fixes the original bugs, the extra "return" and a bit of whitespace.

Looks good. Thanks.

-andmike
--
Michael Anderson
[email protected]