Re: Regression! kern/t_pty fails - cannot set pty's queue size

[Paul Goyette <paul%whooppee.com@localhost>]

On Fri, 17 Aug 2012, Paul Goyette wrote:

> On Fri, 17 Aug 2012, Paul Goyette wrote:
>
> > Some time between 10:20:07 and 15:50:06 UTC on 2012-08-12, the pty_queue 
> > test case for kern/t_pty has been failing.  The error appears to be an 
> > EBUSY return from "ioctl(fd, TIOCSQSIZE, &opt)".  This worked correctly in 
> > prior test runs, but has been failing consistently since Aug 12.
> >
> > This would appear to be a result of the following commit:
> >
> >         Module Name:    src
> >         Committed By:   christos
> >         Date:           Sun Aug 12 14:45:45 UTC 2012
> >
> >         Modified Files:
> >                 src/sys/kern: tty.c
> >
> >         Log Message:
> >         PR/46780: Dennis Ferguson: Take the easy way out and return EBUSY
> >         when changing the queue size if the output queue is not empty.
> >         Other solutions seemed too complex/fragile.
> >
> > Perhaps the "output queue not empty" check isn't working?
>
> The code in question:
>
>         static int
>         tty_set_qsize(struct tty *tp, int newsize)
>         {
>                 struct clist rawq, canq, outq;
>                 struct clist orawq, ocanq, ooutq;
>
>                 if (outq.c_cc != 0)
>                         return EBUSY;
>         ...
>
> It is definitely not obvious to me how outq.c_cc can have any useful value at 
> this point - outq has only just been allocated on the stack.

I propose the attached diffs as a more correct way of fixing this bug. 
Could someone more familiar with this code please review?



-------------------------------------------------------------------------
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------

Index: tty.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty.c,v
retrieving revision 1.251
diff -u -p -r1.251 tty.c
--- tty.c       12 Aug 2012 14:45:44 -0000      1.251
+++ tty.c       17 Aug 2012 13:09:22 -0000
@@ -230,9 +230,6 @@ tty_set_qsize(struct tty *tp, int newsiz
        struct clist rawq, canq, outq;
        struct clist orawq, ocanq, ooutq;
 
-       if (outq.c_cc != 0)
-               return EBUSY;
-
        clalloc(&rawq, newsize, 1);
        clalloc(&canq, newsize, 1);
        clalloc(&outq, newsize, 0);
@@ -243,6 +240,13 @@ tty_set_qsize(struct tty *tp, int newsiz
        ocanq = tp->t_canq;
        ooutq = tp->t_outq;
 
+       if (ooutq.c_cc != 0) {
+               clfree(&rawq);
+               clfree(&canq);
+               clfree(&outq);
+               return EBUSY;
+       }
+
        tp->t_qsize = newsize;
        tp->t_rawq = rawq;
        tp->t_canq = canq;