[2.6 patch] drivers/pci/hotplug/cpqphp_ctrl.c: board_replaced(): remove dead code

[2.6 patch] drivers/pci/hotplug/cpqphp_ctrl.c: board_replaced(): remove dead code

[Adrian Bunk]

he Coverity checker correctly noted, that in function board_replaced in
drivers/pci/hotplug/cpqphp_ctrl.c, the variable src always has the
value 8, and therefore much code after the

...
                        if (rc || src) {
...
                                if (rc)
                                        return rc;
                                else
                                        return 1;
                        }
...


can never be called.

This patch removes the unreachable code in this function fixing kernel 
Bugzilla #6073.


Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 drivers/pci/hotplug/cpqphp_ctrl.c |   76 +++++-------------------------
 1 file changed, 14 insertions(+), 62 deletions(-)

--- linux-2.6.16-rc4-mm2-full/drivers/pci/hotplug/cpqphp_ctrl.c.old	2006-02-26 19:01:10.000000000 +0100
+++ linux-2.6.16-rc4-mm2-full/drivers/pci/hotplug/cpqphp_ctrl.c	2006-02-26 19:07:44.000000000 +0100
@@ -1282,9 +1282,7 @@
 	u8 hp_slot;
 	u8 temp_byte;
 	u8 adapter_speed;
-	u32 index;
 	u32 rc = 0;
-	u32 src = 8;
 
 	hp_slot = func->device - ctrl->slot_device_offset;
 
@@ -1368,68 +1366,17 @@
 
 			rc = cpqhp_configure_board(ctrl, func);
 
-			if (rc || src) {
-				/* If configuration fails, turn it off
-				 * Get slot won't work for devices behind
-				 * bridges, but in this case it will always be
-				 * called for the "base" bus/dev/func of an
-				 * adapter. */
-
-				mutex_lock(&ctrl->crit_sect);
-
-				amber_LED_on (ctrl, hp_slot);
-				green_LED_off (ctrl, hp_slot);
-				slot_disable (ctrl, hp_slot);
-
-				set_SOGO(ctrl);
-
-				/* Wait for SOBS to be unset */
-				wait_for_ctrl_irq (ctrl);
-
-				mutex_unlock(&ctrl->crit_sect);
-
-				if (rc)
-					return rc;
-				else
-					return 1;
-			}
-
-			func->status = 0;
-			func->switch_save = 0x10;
-
-			index = 1;
-			while (((func = cpqhp_slot_find(func->bus, func->device, index)) != NULL) && !rc) {
-				rc |= cpqhp_configure_board(ctrl, func);
-				index++;
-			}
-
-			if (rc) {
-				/* If configuration fails, turn it off
-				 * Get slot won't work for devices behind
-				 * bridges, but in this case it will always be
-				 * called for the "base" bus/dev/func of an
-				 * adapter. */
-
-				mutex_lock(&ctrl->crit_sect);
-
-				amber_LED_on (ctrl, hp_slot);
-				green_LED_off (ctrl, hp_slot);
-				slot_disable (ctrl, hp_slot);
-
-				set_SOGO(ctrl);
-
-				/* Wait for SOBS to be unset */
-				wait_for_ctrl_irq (ctrl);
-
-				mutex_unlock(&ctrl->crit_sect);
-
-				return rc;
-			}
-			/* Done configuring so turn LED on full time */
+			/* If configuration fails, turn it off
+			 * Get slot won't work for devices behind
+			 * bridges, but in this case it will always be
+			 * called for the "base" bus/dev/func of an
+			 * adapter. */
 
 			mutex_lock(&ctrl->crit_sect);
 
-			green_LED_on (ctrl, hp_slot);
+			amber_LED_on (ctrl, hp_slot);
+			green_LED_off (ctrl, hp_slot);
+			slot_disable (ctrl, hp_slot);
 
 			set_SOGO(ctrl);
 
@@ -1437,7 +1384,12 @@
 			wait_for_ctrl_irq (ctrl);
 
 			mutex_unlock(&ctrl->crit_sect);
-			rc = 0;
+
+			if (rc)
+				return rc;
+			else
+				return 1;
+
 		} else {
 			/* Something is wrong
 

Re: [2.6 patch] drivers/pci/hotplug/cpqphp_ctrl.c: board_replaced(): remove dead code

[Matthew Wilcox]

On Sun, Feb 26, 2006 at 10:16:51PM +0100, Adrian Bunk wrote:
> he Coverity checker correctly noted, that in function board_replaced in
> drivers/pci/hotplug/cpqphp_ctrl.c, the variable src always has the
> value 8, and therefore much code after the
> 
> ...
>                         if (rc || src) {
> ...
>                                 if (rc)
>                                         return rc;
>                                 else
>                                         return 1;
>                         }
> ...
> 
> 
> can never be called.
> 
> This patch removes the unreachable code in this function fixing kernel 
> Bugzilla #6073.

It seems much  more  likely to me that the '|| src' indicates a bug, and
the unreachable  code  should have been  reached.  Why not cc the
maintainer for his opinion?

Re: [2.6 patch] drivers/pci/hotplug/cpqphp_ctrl.c: board_replaced(): remove dead code

[Adrian Bunk]

On Sun, Feb 26, 2006 at 04:55:49PM -0700, Matthew Wilcox wrote:
> On Sun, Feb 26, 2006 at 10:16:51PM +0100, Adrian Bunk wrote:
> > he Coverity checker correctly noted, that in function board_replaced in
> > drivers/pci/hotplug/cpqphp_ctrl.c, the variable src always has the
> > value 8, and therefore much code after the
> > 
> > ...
> >                         if (rc || src) {
> > ...
> >                                 if (rc)
> >                                         return rc;
> >                                 else
> >                                         return 1;
> >                         }
> > ...
> > 
> > 
> > can never be called.
> > 
> > This patch removes the unreachable code in this function fixing kernel 
> > Bugzilla #6073.
> 
> It seems much  more  likely to me that the '|| src' indicates a bug, and
> the unreachable  code  should have been  reached.  Why not cc the
> maintainer for his opinion?

I did, at least I thought I did...

I thought the following was the right entry in MAINTAINERS for this 
driver:

PCI HOTPLUG COMPAQ DRIVER
P:      Greg Kroah-Hartman
M:      greg@kroah.com
S:      Maintained


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] drivers/pci/hotplug/cpqphp_ctrl.c: board_replaced(): remove dead code

[Matthew Wilcox]

On Mon, Feb 27, 2006 at 04:12:52PM +0100, Adrian Bunk wrote:
> > Why not cc the
> > maintainer for his opinion?
> 
> I did, at least I thought I did...

My  mistake; I  misread  cpq as compact.    Sorry.