powerpc: hv{cs,si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
unsigned hp->count and hvcsd->open_count cannot be negative
---
Both members of respectively struct hvcs_struct, see vi drivers/char/hvcs.c +262
and struct hvcs_struct, see
vi drivers/char/hvsi.c +70
drivers/char/hvcs.c +5 -2
@@ -1222,7 +1222,8 @@ static void hvcs_close(struct tty_struct *tty, struct file *filp) hvcsd = tty->driver_data;spin_lock_irqsave (&hvcsd->lock, flags); -if (--hvcsd->open_count ==0 ) { +if (hvcsd->open_count ==1 ) { + hvcsd->open_count--;vio_disable_interrupts (hvcsd->vdev);
@@ -1248,7 +1249,9 @@ static void hvcs_close(struct tty_struct *tty, struct file *filp)free_irq (irq, hvcsd);kref_put (&hvcsd->kref, destroy_hvcs_struct);return ; - }else if (hvcsd->open_count <0 ) { + }else if (hvcsd->open_count >1 ) { + hvcsd->open_count--; + }else {printk (KERN_ERR "HVCS: vty-server@%X open_count: %d" " is missmanaged.\n" , hvcsd->vdev->unit_address, hvcsd->open_count);
drivers/char/hvsi.c +5 -2
@@ -875,7 +875,8 @@ static void hvsi_close(struct tty_struct *tty, struct file *filp)spin_lock_irqsave (&hp->lock, flags); -if (--hp->count ==0 ) { +if (hp->count ==1 ) { + hp->count--; hp->tty =NULL ; hp->inbuf_end = hp->inbuf;/* discard remaining partial packets */
@@ -908,7 +909,9 @@ static void hvsi_close(struct tty_struct *tty, struct file *filp)spin_lock_irqsave (&hp->lock, flags); } - }else if (hp->count <0 ) + }else if (hp->count >1 ) + hp->count--; +else printk (KERN_ERR "hvsi_close %lu: oops, count is %d\n" , hp - hvsi_ports, hp->count);
drivers/char/hvcs.c static void hvcs_close(struct tty_struct *tty, struct file *filp) · line 1226
Quoted 3 lines · roel kluin
drivers/char/hvcs.c +2
-1-if (--hvcsd->open_count ==0 ) { +if (hvcsd->open_count ==1 ) { + hvcsd->open_count--;
This is not the same.
drivers/char/hvsi.c static void hvsi_close(struct tty_struct *tty, struct file *filp) · line 879
Quoted 3 lines
drivers/char/hvsi.c +2
-1-if (--hp->count ==0 ) { +if (hp->count ==1 ) { + hp->count--;
Likewise.
Andreas.
Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Quoted 7 lines · Andreas Schwab
drivers/char/hvcs.c static void hvcs_close(struct tty_struct *tty, struct file *filp) · line 1226
Quoted 3 lines · roel kluin
drivers/char/hvcs.c +2
-1-if (--hvcsd->open_count ==0 ) { +if (hvcsd->open_count ==1 ) { + hvcsd->open_count--;This is not the same.
I think you're missing that I also decrement if (hvcsd->open_count > 1) If not, please elaborate.
Quoted 6 lines
drivers/char/hvsi.c static void hvsi_close(struct tty_struct *tty, struct file *filp) · line 879
Quoted 4 lines
drivers/char/hvsi.c +2
-1-if (--hp->count ==0 ) { +if (hp->count ==1 ) { + hp->count--;Likewise.
Same here.
Roel
Quoted 10 lines · roel kluin
Quoted 7 lines · Andreas Schwab
drivers/char/hvcs.c static void hvcs_close(struct tty_struct *tty, struct file *filp) · line 1226
Quoted 3 lines · roel kluin
drivers/char/hvcs.c +2
-1-if (--hvcsd->open_count ==0 ) { +if (hvcsd->open_count ==1 ) { + hvcsd->open_count--;This is not the same.
I think you're missing that I also decrement if (
hvcsd->open_count> 1)
Yes, sorry, I missed that.
Andreas.
Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
In reply to roel kluinQuoted 1 line · roel kluin
unsigned
hp->countandhvcsd->open_countcannot be negative
...
drivers/char/hvcs.c static void hvcs_close(struct tty_struct *tty, struct file *filp) · line 1226
Quoted 3 lines
drivers/char/hvcs.c +2
-1-if (--hvcsd->open_count ==0 ) { +if (hvcsd->open_count ==1 ) { + hvcsd->open_count--;
Why are we no longer decrementing hvcsd->open_count in the cases where it is greater than 1?
drivers/char/hvsi.c static void hvsi_close(struct tty_struct *tty, struct file *filp) · line 879
Quoted 3 lines
drivers/char/hvsi.c +2
-1-if (--hp->count ==0 ) { +if (hp->count ==1 ) { + hp->count--;
Ditto with hp->count here?
Also, I don't see why those changes have anything to do with "unsigned things cannot be negative". As long as those counts are never zero on entry to those code sections, the existing code is fine, and I believe that assertion can be maintained. If you believe the code needs to defend against the possibility of a zero count on entry, that should have been explicitly stated in the patch description.
Paul.
Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
Quoted 6 lines
Also, I don't see why those changes have anything to do with "unsigned things cannot be negative". As long as those counts are never zero on entry to those code sections, the existing code is fine, and I believe that assertion can be maintained. If you believe the code needs to defend against the possibility of a zero count on entry, that should have been explicitly stated in the patch description.
See the tty_port patches in -next - this is an argument about code which ought eventually to go away replaced by standard helper functions.
Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
Quoted 2 lines · Alan Cox
See the
tty_portpatches in -next - this is an argument about code which ought eventually to go away replaced by standard helper functions.
OK. In that case I think the best fix for the existing code is just to change the count to be int rather than unsigned int.
Paul.