The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] alsa: fix comparison of unsigned < 0
@ 2007-05-30  7:35 Bill Nottingham
  2007-05-30 10:16 ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Bill Nottingham @ 2007-05-30  7:35 UTC (permalink / raw)
  To: linux-kernel

Recent gccs emit warnings when unsigned variables are compared < 0 or >= 0.

Signed-off-by: Bill Nottingham <notting@redhat.com>

 ac97/ac97_patch.c    |    3 +--
 ali5451/ali5451.c    |    6 ++----
 ca0106/ca0106_proc.c |    4 ++--
 rme9652/rme9652.c    |    2 --
 4 files changed, 5 insertions(+), 10 deletions(-)

diff -ru linux-2.6.21-old/sound/pci/ac97/ac97_patch.c linux-2.6.21/sound/pci/ac97/ac97_patch.c
--- linux-2.6.21-old/sound/pci/ac97/ac97_patch.c	2007-05-30 02:53:05.000000000 -0400
+++ linux-2.6.21/sound/pci/ac97/ac97_patch.c	2007-05-30 02:32:41.000000000 -0400
@@ -2086,8 +2086,7 @@
 	struct snd_ac97 *ac97 = snd_kcontrol_chip(kcontrol);
 	unsigned short val;
 
-	if (ucontrol->value.enumerated.item[0] > 3
-	    || ucontrol->value.enumerated.item[0] < 0)
+	if (ucontrol->value.enumerated.item[0] > 3)
 		return -EINVAL;
 	val = ctrl2reg[ucontrol->value.enumerated.item[0]]
 	      << AC97_AD198X_VREF_SHIFT;
diff -ru linux-2.6.21-old/sound/pci/ali5451/ali5451.c linux-2.6.21/sound/pci/ali5451/ali5451.c
--- linux-2.6.21-old/sound/pci/ali5451/ali5451.c	2007-05-30 02:53:05.000000000 -0400
+++ linux-2.6.21/sound/pci/ali5451/ali5451.c	2007-05-30 02:33:27.000000000 -0400
@@ -2057,10 +2057,8 @@
 {
 	if (codec->hw_initialized)
 		snd_ali_disable_address_interrupt(codec);
-	if (codec->irq >= 0) {
-		synchronize_irq(codec->irq);
-		free_irq(codec->irq, codec);
-	}
+	synchronize_irq(codec->irq);
+	free_irq(codec->irq, codec);
 	if (codec->port)
 		pci_release_regions(codec->pci);
 	pci_disable_device(codec->pci);
diff -ru linux-2.6.21-old/sound/pci/ca0106/ca0106_proc.c linux-2.6.21/sound/pci/ca0106/ca0106_proc.c
--- linux-2.6.21-old/sound/pci/ca0106/ca0106_proc.c	2007-05-30 02:53:05.000000000 -0400
+++ linux-2.6.21/sound/pci/ca0106/ca0106_proc.c	2007-05-30 02:34:19.000000000 -0400
@@ -305,7 +305,7 @@
         while (!snd_info_get_line(buffer, line, sizeof(line))) {
                 if (sscanf(line, "%x %x", &reg, &val) != 2)
                         continue;
-                if ((reg < 0x40) && (reg >=0) && (val <= 0xffffffff) ) {
+                if ((reg < 0x40) && (val <= 0xffffffff) ) {
 			spin_lock_irqsave(&emu->emu_lock, flags);
 			outl(val, emu->port + (reg & 0xfffffffc));
 			spin_unlock_irqrestore(&emu->emu_lock, flags);
@@ -406,7 +406,7 @@
         while (!snd_info_get_line(buffer, line, sizeof(line))) {
                 if (sscanf(line, "%x %x %x", &reg, &channel_id, &val) != 3)
                         continue;
-                if ((reg < 0x80) && (reg >=0) && (val <= 0xffffffff) && (channel_id >=0) && (channel_id <= 3) )
+                if ((reg < 0x80) && (val <= 0xffffffff) && (channel_id <= 3) )
                         snd_ca0106_ptr_write(emu, reg, channel_id, val);
         }
 }
diff -ru linux-2.6.21-old/sound/pci/rme9652/rme9652.c linux-2.6.21/sound/pci/rme9652/rme9652.c
--- linux-2.6.21-old/sound/pci/rme9652/rme9652.c	2007-05-30 02:53:05.000000000 -0400
+++ linux-2.6.21/sound/pci/rme9652/rme9652.c	2007-05-30 02:35:16.000000000 -0400
@@ -406,8 +406,6 @@
 		} else if (!frag)
 			return 0;
 		offset -= rme9652->max_jitter;
-		if (offset < 0)
-			offset += period_size * 2;
 	} else {
 		if (offset > period_size + rme9652->max_jitter) {
 			if (!frag)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] alsa: fix comparison of unsigned < 0
  2007-05-30  7:35 [PATCH] alsa: fix comparison of unsigned < 0 Bill Nottingham
@ 2007-05-30 10:16 ` Takashi Iwai
  2007-05-30 11:47   ` Jiri Slaby
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2007-05-30 10:16 UTC (permalink / raw)
  To: Bill Nottingham; +Cc: linux-kernel

Hi,

thanks for checking this.
They seem actually hitting some real bugs.


At Wed, 30 May 2007 03:35:54 -0400,
Bill Nottingham wrote:
> 
> diff -ru linux-2.6.21-old/sound/pci/ac97/ac97_patch.c linux-2.6.21/sound/pci/ac97/ac97_patch.c
> --- linux-2.6.21-old/sound/pci/ac97/ac97_patch.c	2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/ac97/ac97_patch.c	2007-05-30 02:32:41.000000000 -0400
> @@ -2086,8 +2086,7 @@
>  	struct snd_ac97 *ac97 = snd_kcontrol_chip(kcontrol);
>  	unsigned short val;
>  
> -	if (ucontrol->value.enumerated.item[0] > 3
> -	    || ucontrol->value.enumerated.item[0] < 0)
> +	if (ucontrol->value.enumerated.item[0] > 3)
>  		return -EINVAL;
>  	val = ctrl2reg[ucontrol->value.enumerated.item[0]]
>  	      << AC97_AD198X_VREF_SHIFT;

This one is fine.


> diff -ru linux-2.6.21-old/sound/pci/ali5451/ali5451.c linux-2.6.21/sound/pci/ali5451/ali5451.c
> --- linux-2.6.21-old/sound/pci/ali5451/ali5451.c	2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/ali5451/ali5451.c	2007-05-30 02:33:27.000000000 -0400
> @@ -2057,10 +2057,8 @@
>  {
>  	if (codec->hw_initialized)
>  		snd_ali_disable_address_interrupt(codec);
> -	if (codec->irq >= 0) {
> -		synchronize_irq(codec->irq);
> -		free_irq(codec->irq, codec);
> -	}
> +	synchronize_irq(codec->irq);
> +	free_irq(codec->irq, codec);
>  	if (codec->port)
>  		pci_release_regions(codec->pci);
>  	pci_disable_device(codec->pci);

Bah, the check isn't wrong but the type of codec->irq.
It should be int instead of unsigned long.  I'll fix it.


> diff -ru linux-2.6.21-old/sound/pci/ca0106/ca0106_proc.c linux-2.6.21/sound/pci/ca0106/ca0106_proc.c
> --- linux-2.6.21-old/sound/pci/ca0106/ca0106_proc.c	2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/ca0106/ca0106_proc.c	2007-05-30 02:34:19.000000000 -0400
> @@ -305,7 +305,7 @@
>          while (!snd_info_get_line(buffer, line, sizeof(line))) {
>                  if (sscanf(line, "%x %x", &reg, &val) != 2)
>                          continue;
> -                if ((reg < 0x40) && (reg >=0) && (val <= 0xffffffff) ) {
> +                if ((reg < 0x40) && (val <= 0xffffffff) ) {

It's a 32bit unsigned int, so the check of 0xffffffff isn't needed,
too.

>  			spin_lock_irqsave(&emu->emu_lock, flags);
>  			outl(val, emu->port + (reg & 0xfffffffc));
>  			spin_unlock_irqrestore(&emu->emu_lock, flags);
> @@ -406,7 +406,7 @@
>          while (!snd_info_get_line(buffer, line, sizeof(line))) {
>                  if (sscanf(line, "%x %x %x", &reg, &channel_id, &val) != 3)
>                          continue;
> -                if ((reg < 0x80) && (reg >=0) && (val <= 0xffffffff) && (channel_id >=0) && (channel_id <= 3) )
> +                if ((reg < 0x80) && (val <= 0xffffffff) && (channel_id <= 3) )
>                          snd_ca0106_ptr_write(emu, reg, channel_id, val);
>          }
>  }

Ditto.


> diff -ru linux-2.6.21-old/sound/pci/rme9652/rme9652.c linux-2.6.21/sound/pci/rme9652/rme9652.c
> --- linux-2.6.21-old/sound/pci/rme9652/rme9652.c	2007-05-30 02:53:05.000000000 -0400
> +++ linux-2.6.21/sound/pci/rme9652/rme9652.c	2007-05-30 02:35:16.000000000 -0400
> @@ -406,8 +406,6 @@
>  		} else if (!frag)
>  			return 0;
>  		offset -= rme9652->max_jitter;
> -		if (offset < 0)
> -			offset += period_size * 2;

I think this check is actually needed, but it must be cast to int.
Will fix this, too.


Takashi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] alsa: fix comparison of unsigned < 0
  2007-05-30 10:16 ` Takashi Iwai
@ 2007-05-30 11:47   ` Jiri Slaby
  2007-05-30 13:16     ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Slaby @ 2007-05-30 11:47 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Bill Nottingham, linux-kernel

Takashi Iwai napsal(a):
> Hi,
> 
> thanks for checking this.
> They seem actually hitting some real bugs.

Bug fixes are ok, but keep on your mind this (I haven't checked if this is
the case):
http://lkml.org/lkml/2006/11/28/206

> At Wed, 30 May 2007 03:35:54 -0400,
> Bill Nottingham wrote:
>> diff -ru linux-2.6.21-old/sound/pci/ac97/ac97_patch.c linux-2.6.21/sound/pci/ac97/ac97_patch.c
>> --- linux-2.6.21-old/sound/pci/ac97/ac97_patch.c	2007-05-30 02:53:05.000000000 -0400
>> +++ linux-2.6.21/sound/pci/ac97/ac97_patch.c	2007-05-30 02:32:41.000000000 -0400
>> @@ -2086,8 +2086,7 @@
>>  	struct snd_ac97 *ac97 = snd_kcontrol_chip(kcontrol);
>>  	unsigned short val;
>>  
>> -	if (ucontrol->value.enumerated.item[0] > 3
>> -	    || ucontrol->value.enumerated.item[0] < 0)
>> +	if (ucontrol->value.enumerated.item[0] > 3)
>>  		return -EINVAL;
>>  	val = ctrl2reg[ucontrol->value.enumerated.item[0]]
>>  	      << AC97_AD198X_VREF_SHIFT;
> 
> This one is fine.
[...]

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] alsa: fix comparison of unsigned < 0
  2007-05-30 11:47   ` Jiri Slaby
@ 2007-05-30 13:16     ` Takashi Iwai
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2007-05-30 13:16 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Bill Nottingham, linux-kernel

At Wed, 30 May 2007 13:47:44 +0200,
Jiri Slaby wrote:
> 
> Takashi Iwai napsal(a):
> > Hi,
> > 
> > thanks for checking this.
> > They seem actually hitting some real bugs.
> 
> Bug fixes are ok, but keep on your mind this (I haven't checked if this is
> the case):
> http://lkml.org/lkml/2006/11/28/206

Yep, I already committed real bugfixes but didn't apply the rest.


thanks,

Takashi

> 
> > At Wed, 30 May 2007 03:35:54 -0400,
> > Bill Nottingham wrote:
> >> diff -ru linux-2.6.21-old/sound/pci/ac97/ac97_patch.c linux-2.6.21/sound/pci/ac97/ac97_patch.c
> >> --- linux-2.6.21-old/sound/pci/ac97/ac97_patch.c	2007-05-30 02:53:05.000000000 -0400
> >> +++ linux-2.6.21/sound/pci/ac97/ac97_patch.c	2007-05-30 02:32:41.000000000 -0400
> >> @@ -2086,8 +2086,7 @@
> >>  	struct snd_ac97 *ac97 = snd_kcontrol_chip(kcontrol);
> >>  	unsigned short val;
> >>  
> >> -	if (ucontrol->value.enumerated.item[0] > 3
> >> -	    || ucontrol->value.enumerated.item[0] < 0)
> >> +	if (ucontrol->value.enumerated.item[0] > 3)
> >>  		return -EINVAL;
> >>  	val = ctrl2reg[ucontrol->value.enumerated.item[0]]
> >>  	      << AC97_AD198X_VREF_SHIFT;
> > 
> > This one is fine.
> [...]
> 
> regards,
> -- 
> http://www.fi.muni.cz/~xslaby/            Jiri Slaby
> faculty of informatics, masaryk university, brno, cz
> e-mail: jirislaby gmail com, gpg pubkey fingerprint:
>  B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-05-30 13:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-30  7:35 [PATCH] alsa: fix comparison of unsigned < 0 Bill Nottingham
2007-05-30 10:16 ` Takashi Iwai
2007-05-30 11:47   ` Jiri Slaby
2007-05-30 13:16     ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox