[PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Linas Vepstas]

Cleanup the flow of control for rpaphp_add_slot(), so as to
make it easier to read. The ext patch will fix a bug in this 
same code. 

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |   40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 11:10:08.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-03 11:03:01.000000000 -0500
@@ -299,32 +299,32 @@ int rpaphp_add_slot(struct device_node *
 	const int *indexes, *names, *types, *power_domains;
 	char *name, *type;
 
+	if (!dn->name || strcmp(dn->name, "pci"))
+		return 0;
+
+	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
+		return 0;
+
 	dbg("Entry %s: dn->full_name=%s\n", __FUNCTION__, dn->full_name);
 
 	/* register PCI devices */
-	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {
-		if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
-			goto exit;
-
-		name = (char *) &names[1];
-		type = (char *) &types[1];
-		for (i = 0; i < indexes[0]; i++,
-	     		name += (strlen(name) + 1), type += (strlen(type) + 1)) 		{
-
-			if (!(slot = alloc_slot_struct(dn, indexes[i + 1], name,
-				       power_domains[i + 1]))) {
-				retval = -ENOMEM;
-				goto exit;
-			}
-			slot->type = simple_strtoul(type, NULL, 10);
+	name = (char *) &names[1];
+	type = (char *) &types[1];
+	for (i = 0; i < indexes[0]; i++) {
+
+		slot = alloc_slot_struct(dn, indexes[i + 1], name, power_domains[i + 1]);
+		if (!slot)
+			return -ENOMEM;
+
+		slot->type = simple_strtoul(type, NULL, 10);
 				
-			dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
-					indexes[i + 1], name, type);
+		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
+				indexes[i + 1], name, type);
 
-			retval = rpaphp_register_pci_slot(slot);
-		}
+		retval = rpaphp_register_pci_slot(slot);
+		name += strlen(name) + 1;
+		type += strlen(type) + 1;
 	}
-exit:
 	dbg("%s - Exit: num_slots=%d rc[%d]\n",
 	    __FUNCTION__, num_slots, retval);
 	return retval;

[PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Linas Vepstas]

Cleanup the flow of control for rpaphp_add_slot(), so as to
make it easier to read. The ext patch will fix a bug in this 
same code. 

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |   40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-28 11:10:08.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-04-03 11:03:01.000000000 -0500
@@ -299,32 +299,32 @@ int rpaphp_add_slot(struct device_node *
 	const int *indexes, *names, *types, *power_domains;
 	char *name, *type;
 
+	if (!dn->name || strcmp(dn->name, "pci"))
+		return 0;
+
+	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
+		return 0;
+
 	dbg("Entry %s: dn->full_name=%s\n", __FUNCTION__, dn->full_name);
 
 	/* register PCI devices */
-	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {
-		if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
-			goto exit;
-
-		name = (char *) &names[1];
-		type = (char *) &types[1];
-		for (i = 0; i < indexes[0]; i++,
-	     		name += (strlen(name) + 1), type += (strlen(type) + 1)) 		{
-
-			if (!(slot = alloc_slot_struct(dn, indexes[i + 1], name,
-				       power_domains[i + 1]))) {
-				retval = -ENOMEM;
-				goto exit;
-			}
-			slot->type = simple_strtoul(type, NULL, 10);
+	name = (char *) &names[1];
+	type = (char *) &types[1];
+	for (i = 0; i < indexes[0]; i++) {
+
+		slot = alloc_slot_struct(dn, indexes[i + 1], name, power_domains[i + 1]);
+		if (!slot)
+			return -ENOMEM;
+
+		slot->type = simple_strtoul(type, NULL, 10);
 				
-			dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
-					indexes[i + 1], name, type);
+		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
+				indexes[i + 1], name, type);
 
-			retval = rpaphp_register_pci_slot(slot);
-		}
+		retval = rpaphp_register_pci_slot(slot);
+		name += strlen(name) + 1;
+		type += strlen(type) + 1;
 	}
-exit:
 	dbg("%s - Exit: num_slots=%d rc[%d]\n",
 	    __FUNCTION__, num_slots, retval);
 	return retval;

[PATCH 0/19]: RPAPHP pci hotplug cleanup patchbomb

[Linas Vepstas]

Hi Greg, 

Please queue these cleanp patches for 2.6.22; cc'ing akpm.

This is a collection of very small patches that clean up various bits and 
pieces of the RPAPHP hotplug code. They eliminate about 100 lines of code, 
almost without changing any function; there are a few minor bugfixes to 
various error paths, and one memleak fix. Some documentation is added.
The result is, I beleive, slightly more readable, easier to understand
code. In particular, the enable/disable add/remove code paths are now
more obviously symmetrical in thier function.

--linas

[PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Linas Vepstas]

Cleanup the flow of control for rpaphp_add_slot(), so as to
make it easier to read. The ext patch will fix a bug in this 
same code. 

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
Cc: John Rose <johnrose@austin.ibm.com>

----
 drivers/pci/hotplug/rpaphp_core.c |   40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

Index: linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c
===================================================================
--- linux-2.6.21-rc4-git4.orig/drivers/pci/hotplug/rpaphp_core.c	2007-03-22 16:35:39.000000000 -0500
+++ linux-2.6.21-rc4-git4/drivers/pci/hotplug/rpaphp_core.c	2007-03-22 16:46:59.000000000 -0500
@@ -299,32 +299,32 @@ int rpaphp_add_slot(struct device_node *
 	const int *indexes, *names, *types, *power_domains;
 	char *name, *type;
 
+	if (!dn || strcmp(dn->name, "pci"))
+		return 0;
+
+	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
+		return 0;
+	
 	dbg("Entry %s: dn->full_name=%s\n", __FUNCTION__, dn->full_name);
 
 	/* register PCI devices */
-	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {
-		if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
-			goto exit;
-
-		name = (char *) &names[1];
-		type = (char *) &types[1];
-		for (i = 0; i < indexes[0]; i++,
-	     		name += (strlen(name) + 1), type += (strlen(type) + 1)) 		{
-
-			if (!(slot = alloc_slot_struct(dn, indexes[i + 1], name,
-				       power_domains[i + 1]))) {
-				retval = -ENOMEM;
-				goto exit;
-			}
-			slot->type = simple_strtoul(type, NULL, 10);
+	name = (char *) &names[1];
+	type = (char *) &types[1];
+	for (i = 0; i < indexes[0]; i++) {
+
+		slot = alloc_slot_struct(dn, indexes[i + 1], name, power_domains[i + 1]);
+		if (!slot)
+			return -ENOMEM;
+
+		slot->type = simple_strtoul(type, NULL, 10);
 				
-			dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
-					indexes[i + 1], name, type);
+		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
+				indexes[i + 1], name, type);
 
-			retval = rpaphp_register_pci_slot(slot);
-		}
+		retval = rpaphp_register_pci_slot(slot);
+		name += strlen(name) + 1;
+		type += strlen(type) + 1;
 	}
-exit:
 	dbg("%s - Exit: num_slots=%d rc[%d]\n",
 	    __FUNCTION__, num_slots, retval);
 	return retval;

Re: [PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Nathan Lynch]

Linas Vepstas wrote:
> 
> Cleanup the flow of control for rpaphp_add_slot(), so as to
> make it easier to read. The ext patch will fix a bug in this 
> same code. 

This:

> +	if (!dn || strcmp(dn->name, "pci"))
> +		return 0;

is not equivalent to the code it's replacing:

> -	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {

With your version we'll oops if dn->name is NULL.

Re: [PATCH 1/19] PCI: rpaphp: Cleanup flow of control for rpaphp_add_slot

[Linas Vepstas]

On Tue, Apr 03, 2007 at 09:28:28AM -0500, Nathan Lynch wrote:
> Linas Vepstas wrote:
> > 
> > Cleanup the flow of control for rpaphp_add_slot(), so as to
> > make it easier to read. The ext patch will fix a bug in this 
> > same code. 
> 
> This:
> 
> > +	if (!dn || strcmp(dn->name, "pci"))
> > +		return 0;
> 
> is not equivalent to the code it's replacing:
> 
> > -	if (dn->name != 0 && strcmp(dn->name, "pci") == 0) {
> 
> With your version we'll oops if dn->name is NULL.

oops, thanks, I'll fix that.

--linas