Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753146AbXE3KQr (ORCPT ); Wed, 30 May 2007 06:16:47 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751599AbXE3KQk (ORCPT ); Wed, 30 May 2007 06:16:40 -0400 Received: from ns1.suse.de ([195.135.220.2]:33290 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751473AbXE3KQj (ORCPT ); Wed, 30 May 2007 06:16:39 -0400 Date: Wed, 30 May 2007 12:16:38 +0200 Message-ID: From: Takashi Iwai To: Bill Nottingham Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] alsa: fix comparison of unsigned < 0 In-Reply-To: <20070530073554.GA28809@nostromo.devel.redhat.com> References: <20070530073554.GA28809@nostromo.devel.redhat.com> User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 MULE XEmacs/21.5 (beta27) (fiddleheads) (+CVS-20060704) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3646 Lines: 96 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", ®, &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", ®, &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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/