tty_io wtf.

tty_io wtf.

[Dave Jones]

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

Re: tty_io wtf.

[Dave Jones]

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

[PATCH] tty_io.c: keep davej sane

[Alexey Dobriyan]

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 <adobriyan@gmail.com>
---

 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);

Re: [PATCH] tty_io.c: keep davej sane

[Alan Cox]

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 <adobriyan@gmail.com>

Acked-by: Alan Cox <alan@redhat.com>

Re: tty_io wtf.

[David Miller]

From: Dave Jones <davej@redhat.com>
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.