> +#endif
> + tty_insert_flip_char(xencons_tty, buf[i], 0);
Please use the defines like TTY_NORMAL not just 0.
> + if ((xencons_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
> + (xencons_tty->ldisc.write_wakeup != NULL))
> + (xencons_tty->ldisc.write_wakeup)(xencons_tty);
> + }
You are't allowed to derefence and call ldisc methods without
holding the lock. You can replace that chunk with a call to the helper
function tty_wakeup(tty). Small but real race condition otherwise as you
xencons_tty->ldisc may be changing as you call it.
> +static inline int __xencons_put_char(int ch)
> +{
> + char _ch = (char)ch;
> + if ((wp - wc) == wbuf_size)
> + return 0;
> + wbuf[WBUF_MASK(wp++)] = _ch;
> + return 1;
> +}
A lot of very confusing sign stuff here - you turn an int into a char and
put it into a uchar array
> + for (i = 0; i < count; i++)
> + if (!__xencons_put_char(buf[i]))
> + break;
The int coming from a uchar array
Don't think its wrong - just acutely weird and perhaps could be tidier
> +static void xencons_close(struct tty_struct *tty, struct file *filp)
> +{
> + unsigned long flags;
> +
> + mutex_lock(&tty_mutex);
It would be good in future if you could avoid using tty_mutex and use a
private lock. At the moment vt "borrows" it and there are a couple of
incestuous spots but the plan is to eventually fix them and make it
private to tty_io.
Andrew: No objection to this tty stuff being merged provided the bugs
noted above (not worried about the sign stuff) are fixed before it goes
on to Linus.
Alan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
