* powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
@ 2008-11-29 10:57 roel kluin
2008-11-29 12:54 ` Andreas Schwab
2008-11-29 23:20 ` Paul Mackerras
0 siblings, 2 replies; 7+ messages in thread
From: roel kluin @ 2008-11-29 10:57 UTC (permalink / raw)
To: benh, linuxppc-dev, linux-kernel
unsigned hp->count and hvcsd->open_count cannot be negative
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
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
diff --git a/drivers/char/hvcs.c b/drivers/char/hvcs.c
index 473d9b1..b228b84 100644
--- a/drivers/char/hvcs.c
+++ b/drivers/char/hvcs.c
@@ -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);
diff --git a/drivers/char/hvsi.c b/drivers/char/hvsi.c
index 59c6f9a..d46bccd 100644
--- a/drivers/char/hvsi.c
+++ b/drivers/char/hvsi.c
@@ -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);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
2008-11-29 10:57 powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative roel kluin
@ 2008-11-29 12:54 ` Andreas Schwab
2008-11-29 13:27 ` roel kluin
2008-11-29 23:20 ` Paul Mackerras
1 sibling, 1 reply; 7+ messages in thread
From: Andreas Schwab @ 2008-11-29 12:54 UTC (permalink / raw)
To: roel kluin; +Cc: linux-kernel, linuxppc-dev
roel kluin <roel.kluin@gmail.com> writes:
> - if (--hvcsd->open_count == 0) {
> + if (hvcsd->open_count == 1) {
> + hvcsd->open_count--;
This is not the same.
> - if (--hp->count == 0) {
> + if (hp->count == 1) {
> + hp->count--;
Likewise.
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
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."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
2008-11-29 12:54 ` Andreas Schwab
@ 2008-11-29 13:27 ` roel kluin
2008-11-29 15:01 ` Andreas Schwab
0 siblings, 1 reply; 7+ messages in thread
From: roel kluin @ 2008-11-29 13:27 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linux-kernel, linuxppc-dev
Andreas Schwab wrote:
> roel kluin <roel.kluin@gmail.com> writes:
>
>> - 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.
>
>> - if (--hp->count == 0) {
>> + if (hp->count == 1) {
>> + hp->count--;
>
> Likewise.
Same here.
Roel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
2008-11-29 13:27 ` roel kluin
@ 2008-11-29 15:01 ` Andreas Schwab
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Schwab @ 2008-11-29 15:01 UTC (permalink / raw)
To: roel kluin; +Cc: linux-kernel, linuxppc-dev
roel kluin <roel.kluin@gmail.com> writes:
> Andreas Schwab wrote:
>> roel kluin <roel.kluin@gmail.com> writes:
>>
>>> - 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.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
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."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
2008-11-29 10:57 powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative roel kluin
2008-11-29 12:54 ` Andreas Schwab
@ 2008-11-29 23:20 ` Paul Mackerras
2008-11-30 11:16 ` Alan Cox
1 sibling, 1 reply; 7+ messages in thread
From: Paul Mackerras @ 2008-11-29 23:20 UTC (permalink / raw)
To: roel kluin; +Cc: linux-kernel, linuxppc-dev
roel kluin writes:
> unsigned hp->count and hvcsd->open_count cannot be negative
...
> - 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?
> - 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
2008-11-29 23:20 ` Paul Mackerras
@ 2008-11-30 11:16 ` Alan Cox
2008-11-30 21:23 ` Paul Mackerras
0 siblings, 1 reply; 7+ messages in thread
From: Alan Cox @ 2008-11-30 11:16 UTC (permalink / raw)
To: Paul Mackerras; +Cc: roel kluin, linux-kernel, linuxppc-dev
> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative
2008-11-30 11:16 ` Alan Cox
@ 2008-11-30 21:23 ` Paul Mackerras
0 siblings, 0 replies; 7+ messages in thread
From: Paul Mackerras @ 2008-11-30 21:23 UTC (permalink / raw)
To: Alan Cox; +Cc: roel kluin, linux-kernel, linuxppc-dev
Alan Cox writes:
> See the tty_port patches 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-11-30 21:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-29 10:57 powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative roel kluin
2008-11-29 12:54 ` Andreas Schwab
2008-11-29 13:27 ` roel kluin
2008-11-29 15:01 ` Andreas Schwab
2008-11-29 23:20 ` Paul Mackerras
2008-11-30 11:16 ` Alan Cox
2008-11-30 21:23 ` Paul Mackerras
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).