scsi_debug: Convert to use root_device_register() and root_device_unregister()

PATCH Superseded Nicholas A. Bellinger 2 participants 4 messages Sep 6, 2010 – Sep 7, 2010 SCSI

lore

Share & tools

lore

Nicholas A. Bellinger

author

2010-09-06 22:32 UTC #0 lore raw

PATCH 1/1 scsi_debug: Convert to use root_device_register() and root_device_unregister()

From: Nicholas Bellinger <[email protected]>

Hi Doug and Co,

This patch updates the scsi_debug virtual LLD to use root_device_register() and root_device_unregister() from include/linux/device.h instead of device_register() and device_unregister() respectively within scsi_debug_init() and scsi_debug_exit() This simply involved converting the static struct device pseudo_primary into a pointer that is setup by the call to root_device_register().

Thanks to Richard Sharpe and Dmitry Torokhov for their help with this item.

Signed-off-by Nicholas A. Bellinger <[email protected]>

---
 drivers/scsi/scsi_debug.c |   26 ++++++++------------------
 1 files changed, 8 insertions(+), 18 deletions(-)

drivers/scsi/scsi_debug.c +8 ~~-18~~

index b02bdc6..399c430 100644
@@ -3207,23 +3207,14 @@ static void do_remove_driverfs_files(void)
 	driver_remove_file(&sdebug_driverfs_driver, &driver_attr_add_host);
 }
 
-static void pseudo_0_release(struct device *dev)
-{
-	if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
-		printk(KERN_INFO "scsi_debug: pseudo_0_release() called\n");
-}
-
-static struct device pseudo_primary = {
-	.init_name	= "pseudo_0",
-	.release	= pseudo_0_release,
-};
+struct device *pseudo_primary;
 
 static int __init scsi_debug_init(void)
 {
 	unsigned long sz;
 	int host_to_add;
 	int k;
-	int ret;
+	int ret = 0;
 
 	switch (scsi_debug_sector_size) {
 	case  512:

@@ -3352,10 +3343,9 @@ static int __init scsi_debug_init(void)
 			map_region(0, 2);
 	}
 
-	ret = device_register(&pseudo_primary);
-	if (ret < 0) {
-		printk(KERN_WARNING "scsi_debug: device_register error: %d\n",
-			ret);
+	pseudo_primary = root_device_register("pseudo_0");
+	if (!(pseudo_primary)) {
+		printk(KERN_WARNING "scsi_debug: root_device_register() error\n");
 		goto free_vm;
 	}
 	ret = bus_register(&pseudo_lld_bus);

@@ -3402,7 +3392,7 @@ del_files:
 bus_unreg:
 	bus_unregister(&pseudo_lld_bus);
 dev_unreg:
-	device_unregister(&pseudo_primary);
+	root_device_unregister(pseudo_primary);
 free_vm:
 	if (map_storep)
 		vfree(map_storep);

@@ -3423,7 +3413,7 @@ static void __exit scsi_debug_exit(void)
 	do_remove_driverfs_files();
 	driver_unregister(&sdebug_driverfs_driver);
 	bus_unregister(&pseudo_lld_bus);
-	device_unregister(&pseudo_primary);
+	root_device_unregister(pseudo_primary);
 
 	if (dif_storep)
 		vfree(dif_storep);

@@ -3474,7 +3464,7 @@ static int sdebug_add_adapter(void)
         spin_unlock(&sdebug_host_list_lock);
 
         sdbg_host->dev.bus = &pseudo_lld_bus;
-        sdbg_host->dev.parent = &pseudo_primary;
+        sdbg_host->dev.parent = pseudo_primary;
         sdbg_host->dev.release = &sdebug_release_adapter;
         dev_set_name(&sdbg_host->dev, "adapter%d", scsi_debug_add_host);

Signature · 1 line

1.5.6.5

Dmitry Torokhov

2010-09-06 23:41 UTC #1 lore raw

drivers/scsi/scsi_debug.c static void do_remove_driverfs_files(void) · line 3217

Quoted 8 lines · Nicholas A. Bellinger

static int __init scsi_debug_init(void)
{
drivers/scsi/scsi_debug.c +1 -1
 	unsigned long sz;
 	int host_to_add;
 	int k;
-	int ret;
+	int ret = 0;

Please do not initialize error condition with success; when adding additional initialization it makes easy to miss assigning proper return value (as you seem to have) and return success in case of failure.

drivers/scsi/scsi_debug.c static int __init scsi_debug_init(void) · line 3347

Quoted 12 lines

drivers/scsi/scsi_debug.c +2 -4

 	switch (scsi_debug_sector_size) {
 	case  512:

@@ -3352,10 +3343,9 @@ static int __init scsi_debug_init(void)
 			map_region(0, 2);
 	}
 
-	ret = device_register(&pseudo_primary);
-	if (ret < 0) {
-		printk(KERN_WARNING "scsi_debug: device_register error: %d\n",
-			ret);
+	pseudo_primary = root_device_register("pseudo_0");
+	if (!(pseudo_primary)) {

root_device_register() returns ERR_PTR-encoded error codes, you should do:

	if (IS_ERR(pseudo_primary)) {
		printk(KERN_WARNING "scsi_debug: root_device_register() error\n");
		ret = PTR_ERR(pseudo_primary);
		goto free_vm;
	}

Same goes for your other patch.

Thanks.

Signature · 1 line

Dmitry

Nicholas A. Bellinger

author

2010-09-06 23:43 UTC #2 lore raw

Quoted 13 lines · Dmitry Torokhov

drivers/scsi/scsi_debug.c static void do_remove_driverfs_files(void) · line 3217
Quoted 8 lines · Nicholas A. Bellinger
static int __init scsi_debug_init(void)
{
drivers/scsi/scsi_debug.c +1 -1
 	unsigned long sz;
 	int host_to_add;
 	int k;
-	int ret;
+	int ret = 0;
Please do not initialize error condition with success; when adding additional initialization it makes easy to miss assigning proper return value (as you seem to have) and return success in case of failure.

The reason this was added because my gcc (Debian 4.3.2-1.1) 4.3.2 complained about this being possibily uninitialized..

Quoted 17 lines

drivers/scsi/scsi_debug.c static int __init scsi_debug_init(void) · line 3347
Quoted 13 lines
drivers/scsi/scsi_debug.c +2 -4
 	switch (scsi_debug_sector_size) {
 	case  512:
@@ -3352,10 +3343,9 @@ static int __init scsi_debug_init(void)
 			map_region(0, 2);
 	}
 
-	ret = device_register(&pseudo_primary);
-	if (ret < 0) {
-		printk(KERN_WARNING "scsi_debug: device_register error: %d\n",
-			ret);
+	pseudo_primary = root_device_register("pseudo_0");
+	if (!(pseudo_primary)) {
root_device_register() returns ERR_PTR-encoded error codes, you should do:

Quoted 7 lines

	if (IS_ERR(pseudo_primary)) {
		printk(KERN_WARNING "scsi_debug: root_device_register() error\n");
		ret = PTR_ERR(pseudo_primary);
		goto free_vm;
	}

Same goes for your other patch.

<nod> Updating that now.

Thanks,

--nab

Dmitry Torokhov

2010-09-07 00:12 UTC #3 lore raw

Quoted 15 lines · Nicholas A. Bellinger

Quoted 13 lines · Dmitry Torokhov
drivers/scsi/scsi_debug.c static void do_remove_driverfs_files(void) · line 3217
Quoted 8 lines · Nicholas A. Bellinger
static int __init scsi_debug_init(void)
{
drivers/scsi/scsi_debug.c +1 -1
 	unsigned long sz;
 	int host_to_add;
 	int k;
-	int ret;
+	int ret = 0;
Please do not initialize error condition with success; when adding additional initialization it makes easy to miss assigning proper return value (as you seem to have) and return success in case of failure.

Quoted 3 lines

The reason this was added because my gcc (Debian 4.3.2-1.1) 4.3.2 complained about this being possibily uninitialized..

And rightly so - you did not assign a value to it when handling root_device_register() failure. Compiler warnings are generally there for a reason, not just a nuisance that should be shut off without a second thought.

Signature · 1 line

Dmitry