[PATCH 1/4] sound/isa: use unsigned for loop index

[PATCH 1/4] sound/isa: use unsigned for loop index

[Julia Lawall]

From: Julia Lawall <julia@diku.dk>

A few more cases in the spirit of the patch "Trivial: Replacement of always
>0 ints with unsigned ints" submitted by Ricardo Martins <ricardo@scarybox.net>

The transformation was made using the following semantic patch
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@ // find anything that might decrement the variable
identifier i;
expression E;
position p;
@@

  int i@p;
  ...
(
  &i
|
  i--
|
  --i
|
  i-=E
|
  i+=E
)

@x disable decl_init@
identifier r.i;
expression E;
position p1 != r.p;
@@

(
  volatile int i = 0;
|
  volatile int i;
|
+ unsigned
  int i@p1 = 0;
|
+ unsigned
  int i@p1;
)
  <... when != i = E      
(
  i = 0
|
  i = 1
)
  ...>
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---

 sound/isa/es1688/es1688_lib.c      |    2 +-
 sound/isa/es18xx.c                 |    4 ++--
 sound/isa/opti9xx/opti92x-ad1848.c |    2 +-
 sound/isa/sgalaxy.c                |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)


diff -u -p a/sound/isa/es1688/es1688_lib.c b/sound/isa/es1688/es1688_lib.c
--- a/sound/isa/es1688/es1688_lib.c 2008-04-16 13:28:05.000000000 +0200
+++ b/sound/isa/es1688/es1688_lib.c 2008-05-11 12:20:19.000000000 +0200
@@ -101,7 +101,7 @@ static unsigned char snd_es1688_mixer_re
 
 static int snd_es1688_reset(struct snd_es1688 *chip)
 {
-	int i;
+	unsigned int i;
 
 	outb(3, ES1688P(chip, RESET));		/* valid only for ESS chips, SB -> 1 */
 	udelay(10);
diff -u -p a/sound/isa/es18xx.c b/sound/isa/es18xx.c
--- a/sound/isa/es18xx.c 2008-04-16 13:28:05.000000000 +0200
+++ b/sound/isa/es18xx.c 2008-05-11 12:20:16.000000000 +0200
@@ -361,7 +361,7 @@ static inline int snd_es18xx_mixer_writa
 
 static int snd_es18xx_reset(struct snd_es18xx *chip)
 {
-	int i;
+	unsigned int i;
         outb(0x03, chip->port + 0x06);
         inb(chip->port + 0x06);
         outb(0x00, chip->port + 0x06);
@@ -2238,7 +2238,7 @@ static int __devinit snd_es18xx_isa_prob
 		return snd_es18xx_isa_probe1(dev, pdev);
 	} else {
 		static unsigned long possible_ports[] = {0x220, 0x240, 0x260, 0x280};
-		int i;
+		unsigned int i;
 		for (i = 0; i < ARRAY_SIZE(possible_ports); i++) {
 			port[dev] = possible_ports[i];
 			err = snd_es18xx_isa_probe1(dev, pdev);
diff -u -p a/sound/isa/opti9xx/opti92x-ad1848.c b/sound/isa/opti9xx/opti92x-ad1848.c
--- a/sound/isa/opti9xx/opti92x-ad1848.c 2008-04-16 13:28:05.000000000 +0200
+++ b/sound/isa/opti9xx/opti92x-ad1848.c 2008-05-11 12:20:18.000000000 +0200
@@ -1204,7 +1204,7 @@ static int snd_opti93x_capture_close(str
 static void snd_opti93x_init(struct snd_opti93x *chip)
 {
 	unsigned long flags;
-	int i;
+	unsigned int i;
 
 	spin_lock_irqsave(&chip->lock, flags);
 	snd_opti93x_mce_up(chip);
diff -u -p a/sound/isa/sgalaxy.c b/sound/isa/sgalaxy.c
--- a/sound/isa/sgalaxy.c 2008-04-16 13:28:05.000000000 +0200
+++ b/sound/isa/sgalaxy.c 2008-05-11 12:20:18.000000000 +0200
@@ -79,7 +79,7 @@ MODULE_PARM_DESC(dma1, "DMA1 # for Sound
 
 static int snd_sgalaxy_sbdsp_reset(unsigned long port)
 {
-	int i;
+	unsigned int i;
 
 	outb(1, SBP1(port, RESET));
 	udelay(10);

Re: [PATCH 1/4] sound/isa: use unsigned for loop index

[Ricardo Martins]

[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]

On Sun, 11 May 2008 20:43, Alexey Dobriyan wrote:
> On Sun, May 11, 2008 at 02:50:33PM +0200, Julia Lawall wrote:
> > A few more cases in the spirit of the patch "Trivial: Replacement of always
> > >0 ints with unsigned ints" submitted by Ricardo Martins <ricardo@scarybox.net>
> 
> And rationale for those would be ...?

Acccording to the kernel-janitors TODO [1], Jeff Garzik suggested the following:

  2) "unsigned int" is preferred to "int", it generates better asm code
  on all platforms except sh5. This replacement needs to be done manually,
  because often 'int' is required due to negative values -Exxx commonly
  passed as error values.

Since (most) loop counters such as "int i" are always either zero or a positive
number, they are perfect candidates for using unsigned int instead, imho.
It goes without saying, that each case must be considered separately in
case a negative value is indeed needed.

[1] http://kernelnewbies.org/KernelJanitors/Todo

Regards,
-- 
 Ricardo Martins  *  scarybox.net  *  GPG key: 0x1308F1B4

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/4] sound/isa: use unsigned for loop index

[Alexey Dobriyan]

On Sun, May 11, 2008 at 02:50:33PM +0200, Julia Lawall wrote:
> A few more cases in the spirit of the patch "Trivial: Replacement of always
> >0 ints with unsigned ints" submitted by Ricardo Martins <ricardo@scarybox.net>

And rationale for those would be ...?

> --- a/sound/isa/es1688/es1688_lib.c
> +++ b/sound/isa/es1688/es1688_lib.c
> @@ -101,7 +101,7 @@ static unsigned char snd_es1688_mixer_re
>  
>  static int snd_es1688_reset(struct snd_es1688 *chip)
>  {
> -	int i;
> +	unsigned int i;
>  
>  	outb(3, ES1688P(chip, RESET));		/* valid only for ESS chips, SB -> 1 */
>  	udelay(10);

Re: [PATCH 1/4] sound/isa: use unsigned for loop index

[Alexey Dobriyan]

On Sun, May 11, 2008 at 05:08:32PM +0100, Ricardo Martins wrote:
> On Sun, 11 May 2008 20:43, Alexey Dobriyan wrote:
> > On Sun, May 11, 2008 at 02:50:33PM +0200, Julia Lawall wrote:
> > > A few more cases in the spirit of the patch "Trivial: Replacement of always
> > > >0 ints with unsigned ints" submitted by Ricardo Martins <ricardo@scarybox.net>
> > 
> > And rationale for those would be ...?
> 
> Acccording to the kernel-janitors TODO [1], Jeff Garzik suggested the following:
> 
>   2) "unsigned int" is preferred to "int", it generates better asm code
>   on all platforms except sh5. This replacement needs to be done manually,
>   because often 'int' is required due to negative values -Exxx commonly
>   passed as error values.
> 
> Since (most) loop counters such as "int i" are always either zero or a positive
> number, they are perfect candidates for using unsigned int instead, imho.
> It goes without saying, that each case must be considered separately in
> case a negative value is indeed needed.
> 
> [1] http://kernelnewbies.org/KernelJanitors/Todo

So you've checked disassembly in both cases and saw it's better?