: bug in tty_default_put_char()

	Hi,

	Bug : tty_default_put_char() doesn't check the return value of

tty->driver.write(). However, the later may fail if buffers are full.

Solution : It's not obvious what should be done. The attached
patch is certainly wrong, but gives you an idea of what the problem is.

Long story :
User weant to do PPP over IrCOMM. "chat" opens ircomm device
in non blocking mode and write char by char. I suspect that it's using the above call, but can't verify (because it doesn't happen to me). As IrCOMM has not finished its initialisation (the open was non-blocking), it refuses the write and returns 0.
Character dropped, user unhappy, bugs me about it. I'll try to workaround that in IrCOMM.

Regards,

Jean

-----------------------------------------------

diff -u -p linux/drivers/char/tty_io.t1.c linux/drivers/char/tty_io.c
@@ -2021,7 +2021,11 @@ static void initialize_tty_struct(struct
  */
 void tty_default_put_char(struct tty_struct *tty, unsigned char ch)
 {
-       tty->driver.write(tty, 0, &ch, 1);
+       int ret = tty->driver.write(tty, 0, &ch, 1);
+       /* This might fail if the lower layer is already full - Jean II */
+       if (ret == 0)
+               printk(KERN_WARNING "Warning: dev (%s) put_char failed\n",
+                      kdevname(tty->device));
 }
 
 /*

Just check drivers/char/n_tty.c for every occurence of
put_char() and be scared. The problem is to find a practical solution.
For myself, I've added some clever workaround in IrCOMM to
accept data before full setup.

Regards,

Jean

Sure making it return the right errors doesnt fix anything, but it allows you to fix some of it bit by bit.

put_char() is not allowed to fail since the caller should have already checked for buffer space via the write_room() method.

All places look adequately protected in n_tty.c, so I'm not currently sure how Jean's users are seeing this condition; I'd need to have a BUG() showing the call trace of such an event happening.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Hmm.

Every invocation of the put_char() method should be preceded by a check to ensure that there is sufficient space in the drivers buffer (via the drivers write_room() call.) Could you add a BUG() on this condition and get some call traces please?

I don't think that should be necessary. The tty layer is obviously doing something it shouldn't (trying to stuff characters into a buffer of zero size) so it should get fixed.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

	Hi,

	After sending you bogus path and bogus bug reports, I though

it would be time to send you a *real* patch.
Patch below allow to set "uart=none", which is necessary for
FIR hardware. Patch for 2.5.31.

----------------------------------------------

diff -u -p linux/drivers/serial/core.r1.c linux/drivers/serial/core.c
@@ -730,12 +730,17 @@ uart_set_info(struct uart_info *info, st
                 */
                if (port->type != PORT_UNKNOWN)
                        retval = port->ops->request_port(port);
+               else
+                       /* If we set the uart to unknown, we should *always*
+                        * succeed at the test below. - Jean II */
+                       retval = 0;
 
                /*
                 * If we fail to request resources for the
                 * new port, try to restore the old settings.
                 */
                if (retval && old_type != PORT_UNKNOWN) {
+                       DPRINTK("uart_set_info(%d): could not set new serial configuration, reverting to old one\n", MINOR(tty->device) - tty->driver.minor_start);
                        port->iobase = old_iobase;
                        port->type = old_type;
                        port->hub6 = old_hub6;

----------------------------------------------

Regarding the initial bug I reported.
The write is comming from the PPP line discipline. If PPP
can't transmit the data, it just drops it and assume higher layers will retry. This is true for TCP, but not for "chat".
I implemented the workaround, and the user report that it has
fixed his problem.
I've also found a bunch of other bugs related to that in the
process, but I'll spare you the details.

	Have fun...

	Jean

Ok, thanks.

Last time I read the pppd code, chat doesn't talk via the ppp code, but via the serial device itself. I'm still confused about this report, since it could mean something is very wrong somewhere and not knowing where is really bugging me. I really don't like sleeping problems that come back to bite later.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html