[uml-devel] Re: Fw: [patch] uml: terminal cleanup

[Gerd Knorr <kraxel@bytesex.org>]

> > -void register_winch(int fd, void *device_data)
> > +void register_winch(int fd, struct tty_struct *tty)
> >  {
> >   int pid, thread, thread_fd;
> >   int count;
> Hmm, I saw you have put a forward declaration of struct tty_struct, however it 
> does not seem nice anyway... I don't think that using void* is nice either, 
> but both things have pitfalls.

It's enougth to just have "struct tty_struct;" declared, at least for
code paths which don't access the elements of the the struct and just
pass it through (like the usermode code does in that case).  That
shouldn't create much trouble as you don't need the full tty_struct
declaration for the userspace code, but the compiler can still do type
checking on the arguments (thats why I didn't want it being void*).

> > -void line_close(struct line *lines, struct tty_struct *tty)
> > +void line_close(struct tty_struct *tty, struct file * filp)
> >  {
> > - struct line *line;
> > - int n;
> > -
> > - if(tty == NULL) n = 0;
> > - else n = tty->index;
> > - line = &lines[n];
> > + struct line *line = tty->driver_data;
> >
> >   down(&line->sem);
> >   line->count--;
> > -
> > - /* I don't like this, but I can't think of anything better.  What's
> > -  * going on is that the tty is in the process of being closed for
> > -  * the last time.  Its count hasn't been dropped yet, so it's still
> > -  * at 1.  This may happen when line->count != 0 because of the initial
> > -  * console open (without a tty) bumping it up to 1.
> > -  */
> > - if((line->tty != NULL) && (line->tty->count == 1))
> > -  line->tty = NULL;
> > - if(line->count == 0)
> > -  line_disable(line, -1);
> > + if (tty->count == 1) {

Hmm, while looking at this again, I think maybe this should be
"tty->count == 0" (it's _behind_ the line->count-- after all ...).
Guess you are writing me because you are trying to pin down a bug,
maybe that one is it ;)

> > +  line_disable(tty, -1);
> > +  tty->driver_data = NULL;
> > + }
> >   up(&line->sem);
> >  }
> Why did you delete this comment? I don't think you fixed the problem, you just 
> worked it around the other way...

No, it's fixed.

> Or better, you removed the initial console open, if it refers to the item #2 
> above... but the code isn't clean...

Yes, the hackish console open is gone, and thus the hack above which
attempts to care about that on close (but never really worked correctly)
is gone as well.

The old code used to use the stdio console device before it actually
registered the device in the tty layer, this is why these hacks used to
be in there.  Now the console device is registered after the stdio
console is initialized, and all the problems associated with that are
gone ;)

Drawback of that is that is that you don't see any messages until the
stdio console is initialized.  That isn't a big problem if the kernel
survives up to this point as the console code will simply print all
messages buffered up so far as soon as the console device is opened.
But if the kernel dies early you don't see the messages.  In that case
the new stderr console will help, which can be enabled with "stderr=1"
in the kernel cmd line (maybe additionally "console=stderr") and which
is simple enougth that it can register very early in the boot process.

HTH,

  Gerd

-- 
#define printk(args...) fprintf(stderr, ## args)


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel