Dubious loop logic in pciide(4)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Dubious loop logic in pciide(4)

Michael McConville-2
Yesterday, I ran a Coccinelle script to find useless continue statements
and sent a few to tedu@. He pointed out that, based on the loop above
it, the one below probably meant to continue the outer loop.

Here's the least invasive fix, assuming we're correct. Thoughts? I don't
know if this is the nicest solution.


Index: sys/dev/pci/pciide.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/pciide.c,v
retrieving revision 1.354
diff -u -p -r1.354 pciide.c
--- sys/dev/pci/pciide.c 10 Sep 2015 18:10:34 -0000 1.354
+++ sys/dev/pci/pciide.c 30 Sep 2015 14:57:30 -0000
@@ -6968,7 +6968,7 @@ pdcsata_chip_map(struct pciide_softc *sc
     "regs\n",
     sc->sc_wdcdev.sc_dev.dv_xname,
     channel);
- continue;
+ goto loop_end;
  }
  }
  ps->regs[channel].cmd_iohs[wdr_status & _WDC_REGMASK] =
@@ -7014,6 +7014,8 @@ pdcsata_chip_map(struct pciide_softc *sc
     (channel + 1) << 2, 0x00000001);
 
  pdc203xx_setup_channel(&cp->wdc_channel);
+
+loop_end: ;
  }
 
  printf("%s: using %s for native-PCI interrupt\n",

Reply | Threaded
Open this post in threaded view
|

Re: Dubious loop logic in pciide(4)

Mark Kettenis
> Date: Wed, 30 Sep 2015 11:35:36 -0400
> From: Michael McConville <[hidden email]>
>
> Yesterday, I ran a Coccinelle script to find useless continue statements
> and sent a few to tedu@. He pointed out that, based on the loop above
> it, the one below probably meant to continue the outer loop.
>
> Here's the least invasive fix, assuming we're correct. Thoughts? I don't
> know if this is the nicest solution.

Certainly the least invasive.  But I think you should drop the
semicolon after the label.

With that, ok kettenis@

> Index: sys/dev/pci/pciide.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/pciide.c,v
> retrieving revision 1.354
> diff -u -p -r1.354 pciide.c
> --- sys/dev/pci/pciide.c 10 Sep 2015 18:10:34 -0000 1.354
> +++ sys/dev/pci/pciide.c 30 Sep 2015 14:57:30 -0000
> @@ -6968,7 +6968,7 @@ pdcsata_chip_map(struct pciide_softc *sc
>      "regs\n",
>      sc->sc_wdcdev.sc_dev.dv_xname,
>      channel);
> - continue;
> + goto loop_end;
>   }
>   }
>   ps->regs[channel].cmd_iohs[wdr_status & _WDC_REGMASK] =
> @@ -7014,6 +7014,8 @@ pdcsata_chip_map(struct pciide_softc *sc
>      (channel + 1) << 2, 0x00000001);
>  
>   pdc203xx_setup_channel(&cp->wdc_channel);
> +
> +loop_end: ;
>   }
>  
>   printf("%s: using %s for native-PCI interrupt\n",
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Dubious loop logic in pciide(4)

Miod Vallat
> Certainly the least invasive.  But I think you should drop the
> semicolon after the label.

Nope, the semicolon is required by the C standard those days (labels
must be followed by statements).