Re: Enhance ptyfs to handle multiple instances.

[christos%zoulas.com@localhost (Christos Zoulas)]

On Mar 19,  6:09pm, netbsd%izyk.ru@localhost (Ilya Zykov) wrote:
-- Subject: Re: Enhance ptyfs to handle multiple instances.

| Index: nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c
| diff -u nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c:1.1.1.1 
nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c:1.7
| --- nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c:1.1.1.1     Tue Mar  4 22:16:03 2014
| +++ nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c     Wed Mar 19 15:41:47 2014
| @@ -109,14 +109,16 @@
|       buf = malloc(MAXBUF, M_TEMP, M_WAITOK);
|       bp = buf + MAXBUF;
|       *--bp = '\0';
| -     error = getcwd_common(cwdi->cwdi_rdir, rootvnode, &bp,
| +     error = getcwd_common(mp->mnt_vnodecovered, cwdi->cwdi_rdir, &bp,
|           buf, MAXBUF / 2, 0, l);

Might as well pass NULL for cwdi->cwdi_rdir, since it does the same.

| -     if (error)      /* XXX */
| +     if (error) {    /* Mount point is out of rdir */
| +             rv = NULL;
|               goto out;
| +     }

Ok, error is better in this case.

|       len = strlen(bp);
|       if (len < sizeof(mp->mnt_stat.f_mntonname))     /* XXX */
| -             rv += len;
| +             rv += strlen(rv) - len;

Ok.

|  out:
|       free(buf, M_TEMP);
|       return rv;
| @@ -128,6 +130,7 @@
|  {
|       struct mount *mp = pt->arg;
|       size_t len;
| +     const char *np;
|  
|       switch (ms) {
|       case 'p':
| @@ -135,9 +138,13 @@
|               len = snprintf(tbuf, bufsiz, "/dev/null");
|               break;
|       case 't':
| -             len = snprintf(tbuf, bufsiz, "%s/%llu", ptyfs__getpath(l, mp),
| -                 (unsigned long long)minor(dev));
| -             break;
| +             np = ptyfs__getpath(l, mp);
| +             if ( np ) {

Whitespace. I would flip the logic (it reduces the diff too).

                if (np == NULL)
                        return EOPNOTSUPP;

                len = snprintf(tbuf, bufsiz, "%s/%llu", np,
                    (unsigned long long)minor(dev));
                break;
|       default:
|               return EINVAL;
|       }
| @@ -241,7 +248,7 @@
|               return 0;
|       }
|  
| -#if 0
| +#if 1
|       /* Don't allow more than one mount */
|       if (ptyfs_count)
|               return EBUSY;

People did not like that.

| Index: nbcur/src/sys/kern/tty_ptm.c
| diff -u nbcur/src/sys/kern/tty_ptm.c:1.1.1.2 nbcur/src/sys/kern/tty_ptm.c:1.2
| --- nbcur/src/sys/kern/tty_ptm.c:1.1.1.2      Mon Mar 17 15:46:10 2014
| +++ nbcur/src/sys/kern/tty_ptm.c      Wed Mar 19 12:59:47 2014
| @@ -381,7 +381,9 @@
|                       goto bad;
|  
|               /* now, put the indices and names into struct ptmget */
| -             return pty_fill_ptmget(l, newdev, cfd, sfd, data);
| +             if ((error = pty_fill_ptmget(l, newdev, cfd, sfd, data)) != 0 )

Whitespace.

| +                     break;  /* goto bad2 */
| +             return 0;
|       default:
|  #ifdef COMPAT_60
|               error = compat_60_ptmioctl(dev, cmd, data, flag, l);
| @@ -391,6 +393,11 @@
|               DPRINTF(("ptmioctl EINVAL\n"));
|               return EINVAL;
|       }
| +/* bad2: close sfd too */
| +     fp = fd_getfile(sfd);
| +     if (fp != NULL) {
| +             fd_close(sfd);
| +     }
|   bad:
|       fp = fd_getfile(cfd);
|       if (fp != NULL) {

Yes, this is correct.

christos