serial port canonical mode weirdness?

serial port canonical mode weirdness?

[David L]

When I configure a serial port for canonical mode (newtio.c_lflag = ICANON),
I get behavior that isn't what I'd expect.  If I send >4095 characters that 
don't
contain an end-of-line and then an end of line, the number of characters 
read
is 1.  If I then start sending shorter lines, one character is read for each
end-of-line that is sent.  For example, if the limit was 10 instead of 4096, 
if the
received data was:

0123456789\n

The data returned by the driver would be:

0

(I would expect 0123456789\n to be returned)

Then if you send "A\n"
The data returned will be "1"

Then if you send "B\n"
The data returned will be "2"

And so on.

When it's in this state and you send "\n\n", it returns 4096 characters
and then starts acting like I would expect again.

I've also found that when it's in this state and you send
a few lines in rapid succession, the call to read will return 0
bytes and never returns any more good data.

Is this a kernel bug or an ignorant user?  I might expect some
data to be lost if a buffer overruns, but I didn't expect this behavior.

Thanks:

                                           David

_________________________________________________________________
MSN Toolbar provides one-click access to Hotmail from any Web page – FREE 
download! http://toolbar.msn.com/go/onm00200413ave/direct/01/

Re: serial port canonical mode weirdness?

[Russell King]

On Wed, Mar 31, 2004 at 04:44:32PM -0800, David L wrote:
> When I configure a serial port for canonical mode (newtio.c_lflag = ICANON),
> I get behavior that isn't what I'd expect.

Can you supply the test program you're using on the receive end?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: serial port canonical mode weirdness?

[David L]

> > When I configure a serial port for canonical mode (newtio.c_lflag = 
>ICANON),
> > I get behavior that isn't what I'd expect.
>
>Can you supply the test program you're using on the receive end?
>
>--
#include <termios.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>

#define BAUDRATE B38400
#define SERIALDEVICE "/dev/ttyS0"

int main()
{
  int fd, res;
  struct termios newtio;
  unsigned char buf[10000];

  fd = open(SERIALDEVICE, O_RDWR);
  if (fd <0) {perror(SERIALDEVICE); return -1; }

  bzero(&newtio, sizeof(newtio));

  newtio.c_cflag = BAUDRATE | CS8 | CLOCAL | CREAD;
  newtio.c_iflag = IGNPAR;
  newtio.c_lflag = ICANON;

  newtio.c_cc[VEOL]     = 0x0D;

  tcsetattr(fd,TCSANOW,&newtio);

  while (1) {
    res = read(fd,buf,9000);
    printf("%d bytes read (%d)\n", res, errno);
    printf("%02X %02X %02X %02X %02X %02X\n", 
buf[0],buf[1],buf[2],buf[3],buf[4],buf[5]);
  }
}

_________________________________________________________________
FREE pop-up blocking with the new MSN Toolbar – get it now! 
http://toolbar.msn.com/go/onm00200415ave/direct/01/

Re: serial port canonical mode weirdness?

[Russell King]

On Wed, Mar 31, 2004 at 04:44:32PM -0800, David L wrote:
> When I configure a serial port for canonical mode (newtio.c_lflag = ICANON),
> I get behavior that isn't what I'd expect.
>...
> Is this a kernel bug or an ignorant user?  I might expect some
> data to be lost if a buffer overruns, but I didn't expect this behavior.

It's a kernel bug.  There are actually three bugs:

1. when we receive a buffer-full of characters, followed by a new line,
   we mark the first character of the buffer as being the last character
   of that line.  (this is why you get one character returned.)

2. when the new line is received, we have no buffer space to store it.
   with (1) fixed, this results in the canon head never being reached,
   so future read() calls returning zero without waiting.

3. we don't atomically add two and three byte character sequences to the
   buffer, so it's possible to have incomplete sequences in the buffer.

This patch fixes all three (according to my rudimentary testing here),
though please test it anyway.

Please note that this actually means that we can only receive 4094
characters before we overrun the buffer instead of 4096, even in raw
mode.

--- orig/drivers/char/n_tty.c	Sat Feb 28 10:09:05 2004
+++ linux/drivers/char/n_tty.c	Fri Apr  2 22:57:27 2004
@@ -83,24 +83,30 @@ static inline void free_buf(unsigned cha
 		free_page((unsigned long) buf);
 }
 
-static inline void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
+static inline void put_tty_queue_nolock(struct tty_struct *tty, unsigned char *s, int num)
 {
-	if (tty->read_cnt < N_TTY_BUF_SIZE) {
-		tty->read_buf[tty->read_head] = c;
-		tty->read_head = (tty->read_head + 1) & (N_TTY_BUF_SIZE-1);
-		tty->read_cnt++;
+	if (tty->read_cnt + num < N_TTY_BUF_SIZE) {
+		while (num--) {
+			tty->read_buf[tty->read_head] = *s++;
+			tty->read_head = (tty->read_head + 1) & (N_TTY_BUF_SIZE-1);
+			tty->read_cnt++;
+		}
 	}
 }
 
-static inline void put_tty_queue(unsigned char c, struct tty_struct *tty)
+static inline void put_tty_queue(struct tty_struct *tty, unsigned char *s, int num)
 {
 	unsigned long flags;
 	/*
 	 *	The problem of stomping on the buffers ends here.
 	 *	Why didn't anyone see this one coming? --AJK
+	 *
+	 *	We must allow two spare characters for the EOL
+	 *	character(s). --rmk
 	*/
 	spin_lock_irqsave(&tty->read_lock, flags);
-	put_tty_queue_nolock(c, tty);
+	if (tty->read_cnt + num < N_TTY_BUF_SIZE - 2)
+		put_tty_queue_nolock(tty, s, num);
 	spin_unlock_irqrestore(&tty->read_lock, flags);
 }
 
@@ -481,6 +487,9 @@ static inline void isig(int sig, struct 
 
 static inline void n_tty_receive_break(struct tty_struct *tty)
 {
+	unsigned char str[3];
+	int idx = 0;
+
 	if (I_IGNBRK(tty))
 		return;
 	if (I_BRKINT(tty)) {
@@ -488,10 +497,11 @@ static inline void n_tty_receive_break(s
 		return;
 	}
 	if (I_PARMRK(tty)) {
-		put_tty_queue('\377', tty);
-		put_tty_queue('\0', tty);
+		str[idx++] = '\377';
+		str[idx++] = '\0';
 	}
-	put_tty_queue('\0', tty);
+	str[idx++] = '\0';
+	put_tty_queue(tty, str, idx);
 	wake_up_interruptible(&tty->read_wait);
 }
 
@@ -511,26 +521,32 @@ static inline void n_tty_receive_overrun
 static inline void n_tty_receive_parity_error(struct tty_struct *tty,
 					      unsigned char c)
 {
+	unsigned char str[3];
+	int idx = 0;
+
 	if (I_IGNPAR(tty)) {
 		return;
 	}
 	if (I_PARMRK(tty)) {
-		put_tty_queue('\377', tty);
-		put_tty_queue('\0', tty);
-		put_tty_queue(c, tty);
+		str[idx++] = '\377';
+		str[idx++] = '\0';
+		str[idx++] = c;
 	} else	if (I_INPCK(tty))
-		put_tty_queue('\0', tty);
+		str[idx++] = '\0';
 	else
-		put_tty_queue(c, tty);
+		str[idx++] = c;
+	put_tty_queue(tty, str, idx);
 	wake_up_interruptible(&tty->read_wait);
 }
 
 static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
 {
 	unsigned long flags;
+	unsigned char str[3];
+	int idx;
 
 	if (tty->raw) {
-		put_tty_queue(c, tty);
+		put_tty_queue(tty, &c, 1);
 		return;
 	}
 	
@@ -574,9 +590,11 @@ static inline void n_tty_receive_char(st
 				tty->canon_column = tty->column;
 			echo_char(c, tty);
 		}
+		idx = 0;
 		if (I_PARMRK(tty) && c == (unsigned char) '\377')
-			put_tty_queue(c, tty);
-		put_tty_queue(c, tty);
+			str[idx++] = c;
+		str[idx++] = c;
+		put_tty_queue(tty, str, idx);
 		return;
 	}
 		
@@ -613,6 +631,7 @@ send_signal:
 		}
 	}
 	if (tty->icanon) {
+		idx = 0;
 		if (c == ERASE_CHAR(tty) || c == KILL_CHAR(tty) ||
 		    (c == WERASE_CHAR(tty) && L_IEXTEN(tty))) {
 			eraser(c, tty);
@@ -678,12 +697,13 @@ send_signal:
 			 * EOL_CHAR and EOL2_CHAR?
 			 */
 			if (I_PARMRK(tty) && c == (unsigned char) '\377')
-				put_tty_queue(c, tty);
+				str[idx++] = c;
 
 		handle_newline:
+			str[idx++] = c;
 			spin_lock_irqsave(&tty->read_lock, flags);
 			set_bit(tty->read_head, tty->read_flags);
-			put_tty_queue_nolock(c, tty);
+			put_tty_queue_nolock(tty, str, idx);
 			tty->canon_head = tty->read_head;
 			tty->canon_data++;
 			spin_unlock_irqrestore(&tty->read_lock, flags);
@@ -710,10 +730,13 @@ send_signal:
 		}
 	}
 
+	idx = 0;
 	if (I_PARMRK(tty) && c == (unsigned char) '\377')
-		put_tty_queue(c, tty);
+		str[idx++] = c;
+
+	str[idx++] = c;
 
-	put_tty_queue(c, tty);
+	put_tty_queue(tty, str, idx);
 }	
 
 static int n_tty_receive_room(struct tty_struct *tty)

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core