Re: [PATCH] ptmx: adding handshake support

[Alan Cox <alan@lxorguk.ukuu.org.uk>]

> + *  Added support for MCR/MSR, used for serial over ethernet
> + *    -- Sander van Ginkel <sander@van-ginkel.eu>

We've been trying to get rid of these long lists in the code and put them
in the git tree (git whatchanged/git blame show the info rather better)

> +
> +    /* initialize the pointer in case something fails */
> +   
> +    tty->driver_data = NULL;                         
> +    tty->link->driver_data = NULL;

Not needed

  +    /* first time accessing this device, let's create it */
> +
> +    mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);

kzalloc will clear it for you...

> +static int pty_tiocmset(struct tty_struct *tty, struct file 
> *file,unsigned int set, unsigned int clear)
> +{
> +    unsigned int *mcrmsr;
> +
> +    mcrmsr = tty->driver_data;
> +    *mcrmsr=set;
> +    tty->driver_data=mcrmsr;

Why this last assignment ?

> +
> +    case VMCRMSR: /* Set all of the handshake line, even the normally 
> read only */
> +        {
> +        if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned 
> int)))
> +            return -EFAULT;
> +
> +        *mcrmsr=value;

Ok - possibly we shouldn't allow people to set undefined bits but I'm not
sure it matters

> +        tty->driver_data=mcrmsr;

Why ??


I am curious how this is handled by other Unix systems and if there is an
ioctl we can follow from other systems ?


Looks basically ok, coding style is wrong, some odd extra assignments but
I agree entirely with the idea of adding this functionality to keep
remote serial drivers in user space.

I'll try and find out if other Unixes have similar features we can use to
keep API consistency tidy it up and fold it at some point in the next
couple of weeks.

Alan