* [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c @ 2007-11-07 18:34 Roel Kluin 2007-11-07 18:43 ` Ray Lee 0 siblings, 1 reply; 5+ messages in thread From: Roel Kluin @ 2007-11-07 18:34 UTC (permalink / raw) To: lkml If count reaches zero, the loop ends, but the postfix decrement subtracts it. so, testing for 'count == 0' will not work. Signed-off-by: Roel Kluin <12o3l@tiscali.nl> --- diff --git a/sound/oss/trident.c b/sound/oss/trident.c index 96adc47..94b5fb4 100644 --- a/sound/oss/trident.c +++ b/sound/oss/trident.c @@ -2939,7 +2939,7 @@ trident_ac97_set(struct ac97_codec *codec, u8 reg, u16 val) data |= (mask | (reg & AC97_REG_ADDR)); - if (count == 0) { + if (count == -1) { printk(KERN_ERR "trident: AC97 CODEC write timed out.\n"); spin_unlock_irqrestore(&card->lock, flags); return; @@ -2999,7 +2999,7 @@ trident_ac97_get(struct ac97_codec *codec, u8 reg) } while (count--); spin_unlock_irqrestore(&card->lock, flags); - if (count == 0) { + if (count == -1) { printk(KERN_ERR "trident: AC97 CODEC read timed out.\n"); data = 0; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c 2007-11-07 18:34 [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c Roel Kluin @ 2007-11-07 18:43 ` Ray Lee 2007-11-07 18:50 ` Roel Kluin 0 siblings, 1 reply; 5+ messages in thread From: Ray Lee @ 2007-11-07 18:43 UTC (permalink / raw) To: Roel Kluin; +Cc: lkml On Nov 7, 2007 10:34 AM, Roel Kluin <12o3l@tiscali.nl> wrote: > If count reaches zero, the loop ends, but the postfix decrement subtracts it. > so, testing for 'count == 0' will not work. > > Signed-off-by: Roel Kluin <12o3l@tiscali.nl> > --- > diff --git a/sound/oss/trident.c b/sound/oss/trident.c > index 96adc47..94b5fb4 100644 > --- a/sound/oss/trident.c > +++ b/sound/oss/trident.c > @@ -2939,7 +2939,7 @@ trident_ac97_set(struct ac97_codec *codec, u8 reg, u16 val) > > data |= (mask | (reg & AC97_REG_ADDR)); > > - if (count == 0) { > + if (count == -1) { > printk(KERN_ERR "trident: AC97 CODEC write timed out.\n"); > spin_unlock_irqrestore(&card->lock, flags); > return; > @@ -2999,7 +2999,7 @@ trident_ac97_get(struct ac97_codec *codec, u8 reg) > } while (count--); > spin_unlock_irqrestore(&card->lock, flags); > > - if (count == 0) { > + if (count == -1) { > printk(KERN_ERR "trident: AC97 CODEC read timed out.\n"); > data = 0; > } You didn't test this: count is unsigned. Change the loop condition to be --count instead. Ray ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c 2007-11-07 18:43 ` Ray Lee @ 2007-11-07 18:50 ` Roel Kluin 2007-11-07 19:04 ` Ray Lee 0 siblings, 1 reply; 5+ messages in thread From: Roel Kluin @ 2007-11-07 18:50 UTC (permalink / raw) To: Ray Lee; +Cc: lkml Ray Lee wrote: > On Nov 7, 2007 10:34 AM, Roel Kluin <12o3l@tiscali.nl> wrote: >> If count reaches zero, the loop ends, but the postfix decrement subtracts it. >> so, testing for 'count == 0' will not work. >> >> Signed-off-by: Roel Kluin <12o3l@tiscali.nl> >> --- >> diff --git a/sound/oss/trident.c b/sound/oss/trident.c >> index 96adc47..94b5fb4 100644 >> --- a/sound/oss/trident.c >> +++ b/sound/oss/trident.c >> @@ -2939,7 +2939,7 @@ trident_ac97_set(struct ac97_codec *codec, u8 reg, u16 val) >> >> data |= (mask | (reg & AC97_REG_ADDR)); >> >> - if (count == 0) { >> + if (count == -1) { >> printk(KERN_ERR "trident: AC97 CODEC write timed out.\n"); >> spin_unlock_irqrestore(&card->lock, flags); >> return; >> @@ -2999,7 +2999,7 @@ trident_ac97_get(struct ac97_codec *codec, u8 reg) >> } while (count--); >> spin_unlock_irqrestore(&card->lock, flags); >> >> - if (count == 0) { >> + if (count == -1) { >> printk(KERN_ERR "trident: AC97 CODEC read timed out.\n"); >> data = 0; >> } > > You didn't test this: count is unsigned. Change the loop condition to > be --count instead. > > Ray Yep, you're right, here: -- If count reaches zero, the loop ends, but the postfix decrement still subtracts: testing for 'count == 0' will not work. Signed-off-by: Roel Kluin <12o3l@tiscali.nl> --- diff --git a/sound/oss/trident.c b/sound/oss/trident.c index 96adc47..6959ee1 100644 --- a/sound/oss/trident.c +++ b/sound/oss/trident.c @@ -2935,7 +2935,7 @@ trident_ac97_set(struct ac97_codec *codec, u8 reg, u16 val) do { if ((inw(TRID_REG(card, address)) & busy) == 0) break; - } while (count--); + } while (--count); data |= (mask | (reg & AC97_REG_ADDR)); @@ -2996,7 +2996,7 @@ trident_ac97_get(struct ac97_codec *codec, u8 reg) data = inl(TRID_REG(card, address)); if ((data & busy) == 0) break; - } while (count--); + } while (--count); spin_unlock_irqrestore(&card->lock, flags); if (count == 0) { ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c 2007-11-07 18:50 ` Roel Kluin @ 2007-11-07 19:04 ` Ray Lee 2007-11-08 6:06 ` Muli Ben-Yehuda 0 siblings, 1 reply; 5+ messages in thread From: Ray Lee @ 2007-11-07 19:04 UTC (permalink / raw) To: Roel Kluin, Alan Cox, Andrew Morton; +Cc: lkml On Nov 7, 2007 10:50 AM, Roel Kluin <12o3l@tiscali.nl> wrote: > If count reaches zero, the loop ends, but the postfix decrement still subtracts: > testing for 'count == 0' will not work. > > Signed-off-by: Roel Kluin <12o3l@tiscali.nl> > --- > diff --git a/sound/oss/trident.c b/sound/oss/trident.c > index 96adc47..6959ee1 100644 > --- a/sound/oss/trident.c > +++ b/sound/oss/trident.c > @@ -2935,7 +2935,7 @@ trident_ac97_set(struct ac97_codec *codec, u8 reg, u16 val) > do { > if ((inw(TRID_REG(card, address)) & busy) == 0) > break; > - } while (count--); > + } while (--count); > > data |= (mask | (reg & AC97_REG_ADDR)); > > @@ -2996,7 +2996,7 @@ trident_ac97_get(struct ac97_codec *codec, u8 reg) > data = inl(TRID_REG(card, address)); > if ((data & busy) == 0) > break; > - } while (count--); > + } while (--count); > > spin_unlock_irqrestore(&card->lock, flags); > > if (count == 0) { > Thanks, much better. In the future, please also CC: the appropriate maintainers, or Andrew Morton if you're at a loss... Reviewed-by: Ray Lee <ray-lk@madrabbit.org> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c 2007-11-07 19:04 ` Ray Lee @ 2007-11-08 6:06 ` Muli Ben-Yehuda 0 siblings, 0 replies; 5+ messages in thread From: Muli Ben-Yehuda @ 2007-11-08 6:06 UTC (permalink / raw) To: Ray Lee, Andrew Morton; +Cc: Roel Kluin, Alan Cox, lkml On Wed, Nov 07, 2007 at 11:04:41AM -0800, Ray Lee wrote: > On Nov 7, 2007 10:50 AM, Roel Kluin <12o3l@tiscali.nl> wrote: > > If count reaches zero, the loop ends, but the postfix decrement > > still subtracts: testing for 'count == 0' will not work. > > > > Signed-off-by: Roel Kluin <12o3l@tiscali.nl> > > --- > > diff --git a/sound/oss/trident.c b/sound/oss/trident.c > > index 96adc47..6959ee1 100644 > > --- a/sound/oss/trident.c > > +++ b/sound/oss/trident.c > > @@ -2935,7 +2935,7 @@ trident_ac97_set(struct ac97_codec *codec, u8 reg, u16 val) > > do { > > if ((inw(TRID_REG(card, address)) & busy) == 0) > > break; > > - } while (count--); > > + } while (--count); > > > > data |= (mask | (reg & AC97_REG_ADDR)); > > > > @@ -2996,7 +2996,7 @@ trident_ac97_get(struct ac97_codec *codec, u8 reg) > > data = inl(TRID_REG(card, address)); > > if ((data & busy) == 0) > > break; > > - } while (count--); > > + } while (--count); > > > > spin_unlock_irqrestore(&card->lock, flags); > > > > if (count == 0) { > > > > Thanks, much better. In the future, please also CC: the appropriate > maintainers, or Andrew Morton if you're at a loss... Indeed. > Reviewed-by: Ray Lee <ray-lk@madrabbit.org> Acked-by: Muli Ben-Yehuda <muli@il.ibm.com> Andrew, can you please push to Linus? Thanks, Muli ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-11-08 6:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-11-07 18:34 [PATCH] fix incorrect test in trident_ac97_set(); sound/oss/trident.c Roel Kluin 2007-11-07 18:43 ` Ray Lee 2007-11-07 18:50 ` Roel Kluin 2007-11-07 19:04 ` Ray Lee 2007-11-08 6:06 ` Muli Ben-Yehuda
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.