I knew I'd regret digging in the tty code.
Can someone enlighten me as to what this *should* be doing?
int tty_insert_flip_string(struct tty_struct *tty, const unsigned char *chars,
size_t size)
{
....
/* There is a small chance that we need to split the data over
several buffers. If this is the case we must loop */
while (unlikely(size > copied));
return copied;
}
Looping I can understand, but forever ?
Given we're not advancing 'copied', can we just kill that while loop?
Or should we be changing it with each iteration?
Dave
--
http://www.codemonkey.org.uk
On Wed, Aug 02, 2006 at 06:36:04PM -0400, Dave Jones wrote:
> I knew I'd regret digging in the tty code.
> Can someone enlighten me as to what this *should* be doing?
>
> int tty_insert_flip_string(struct tty_struct *tty, const unsigned char *chars,
> size_t size)
> {
> ....
> /* There is a small chance that we need to split the data over
> several buffers. If this is the case we must loop */
> while (unlikely(size > copied));
> return copied;
> }
>
>
> Looping I can understand, but forever ?
> Given we're not advancing 'copied', can we just kill that while loop?
> Or should we be changing it with each iteration?
Disregard, it's the end of a do { } while.
I've been staring at this too long.
Dave
--
http://www.codemonkey.org.uk
From: Dave Jones <[email protected]>
Date: Wed, 2 Aug 2006 18:36:04 -0400
> Looping I can understand, but forever ?
It's a do { } while() loop David.
Yes, that comment is surely poorly placed.
On Wed, Aug 02, 2006 at 06:37:33PM -0400, Dave Jones wrote:
> On Wed, Aug 02, 2006 at 06:36:04PM -0400, Dave Jones wrote:
> > I knew I'd regret digging in the tty code.
> > Can someone enlighten me as to what this *should* be doing?
> >
> > int tty_insert_flip_string(struct tty_struct *tty, const unsigned char *chars,
> > size_t size)
> > {
> > ....
> > /* There is a small chance that we need to split the data over
> > several buffers. If this is the case we must loop */
> > while (unlikely(size > copied));
> > return copied;
> > }
> >
> >
> > Looping I can understand, but forever ?
> > Given we're not advancing 'copied', can we just kill that while loop?
> > Or should we be changing it with each iteration?
>
> Disregard, it's the end of a do { } while.
don't hold back, i need your ack
> I've been staring at this too long.
[PATCH] tty_io.c: keep davej sane
Just comment and next "while" look _very_ wrong. Place { correctly to
hint unsuspecting ones that it's the end of the loop actually.
Signed-off-by: Alexey Dobriyan <[email protected]>
---
drivers/char/tty_io.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -362,10 +362,9 @@ int tty_insert_flip_string(struct tty_st
tb->used += space;
copied += space;
chars += space;
- }
- /* There is a small chance that we need to split the data over
- several buffers. If this is the case we must loop */
- while (unlikely(size > copied));
+ /* There is a small chance that we need to split the data over
+ several buffers. If this is the case we must loop */
+ } while (unlikely(size > copied));
return copied;
}
EXPORT_SYMBOL(tty_insert_flip_string);
@@ -386,10 +385,9 @@ int tty_insert_flip_string_flags(struct
copied += space;
chars += space;
flags += space;
- }
- /* There is a small chance that we need to split the data over
- several buffers. If this is the case we must loop */
- while (unlikely(size > copied));
+ /* There is a small chance that we need to split the data over
+ several buffers. If this is the case we must loop */
+ } while (unlikely(size > copied));
return copied;
}
EXPORT_SYMBOL(tty_insert_flip_string_flags);
Ar Iau, 2006-08-03 am 03:02 +0400, ysgrifennodd Alexey Dobriyan:
> Just comment and next "while" look _very_ wrong. Place { correctly to
> hint unsuspecting ones that it's the end of the loop actually.
>
> Signed-off-by: Alexey Dobriyan <[email protected]>
Acked-by: Alan Cox <[email protected]>