Craig Rodrigues wrote:
rodrigc     2005-11-09 02:26:38 UTC

  FreeBSD src repository

  Modified files:
sys/kern vfs_mount.c Log:
  For nmount(), allow a text string error message to be propagated back
  to user-space if a parameter named "errmsg" is passed into the iovec.
  Used in conjunction with vfs_mount_error(), more useful error messages
  than errno can be passed back to userspace when mounting a filesystem
  fails.
Discussed with: phk, pjd Revision Changes Path
  1.199     +37 -2     src/sys/kern/vfs_mount.c

While I don't have ideas for a better general mechanism for this, I think it sets a bad precedent. We can't have every complex syscall transporting its own error message strings back to the user program. And we can't expand errno to be the union of every single API-specific error either.

Other comments below.

Index: src/sys/kern/vfs_mount.c
diff -u src/sys/kern/vfs_mount.c:1.198 src/sys/kern/vfs_mount.c:1.199
--- src/sys/kern/vfs_mount.c:1.198      Tue Nov  8 04:13:39 2005
+++ src/sys/kern/vfs_mount.c    Wed Nov  9 02:26:38 2005
@@ -390,6 +390,18 @@
                iov++;
        }
        error = vfs_donmount(td, uap->flags, auio);
+
+       /* copyout the errmsg */
+       for (i = 0; (error != 0) && (i < iovcnt); i += 2) {

Extra parens.

+               const char *name = (const char *)auio->uio_iov[i].iov_base;

Unnecessary cast, variable declared in fn body. Why not just use auio->uio_iov[i].iov_base in the strcmp directly?

+               if (!strcmp(name, "errmsg")) {
+                       copyout(auio->uio_iov[i+1].iov_base,
+                           uap->iovp[i+1].iov_base, uap->iovp[i+1].iov_len);
+                       

Extra empty line above.

+                       break;
+               }
+       }
+
        free(auio, M_IOV);
        return (error);
 }
@@ -464,8 +476,15 @@
 vfs_donmount(struct thread *td, int fsflags, struct uio *fsoptions)
 {
        struct vfsoptlist *optlist;
+       struct iovec *iov_errmsg = NULL;

Initialization in declaration.

        char *fstype, *fspath;
        int error, fstypelen, fspathlen;
+       int i;
+
+       for (i = 0; i < fsoptions->uio_iovcnt; i += 2) {
+               if (!strcmp((char *)fsoptions->uio_iov[i].iov_base, "errmsg"))

No need for a cast.

+                       iov_errmsg = &fsoptions->uio_iov[i+1];
+       }
error = vfs_buildopts(fsoptions, &optlist);
        if (error)
@@ -480,12 +499,18 @@
        error = vfs_getopt(optlist, "fstype", (void **)&fstype, &fstypelen);
        if (error || fstype[fstypelen - 1] != '\0') {
                error = EINVAL;
+               if (iov_errmsg)
+                       strncpy((char *)iov_errmsg->iov_base, "Invalid fstype",
+                           iov_errmsg->iov_len);

Is iov_len >= strlen(iov_errmsg->iov_base)? If not, it won't copy the terminating NUL. Also, if iov_len is big, strncpy wastes time zero-filling the tail. Perhaps use strlcpy instead.

                goto bail;
        }
        fspathlen = 0;
        error = vfs_getopt(optlist, "fspath", (void **)&fspath, &fspathlen);
        if (error || fspath[fspathlen - 1] != '\0') {
                error = EINVAL;
+               if (iov_errmsg != NULL)
+                       strncpy((char *)iov_errmsg->iov_base, "Invalid fspath",
+                           iov_errmsg->iov_len);
                goto bail;
        }
@@ -503,6 +528,17 @@
        error = vfs_domount(td, fstype, fspath, fsflags, optlist);
        mtx_unlock(&Giant);
 bail:
+       if (error && iov_errmsg != NULL) {
+               /* save the errmsg */
+               char *errmsg;
+               int len, ret;

Variables declared in fn body.

+ ret = vfs_getopt(optlist, "errmsg", (void **)&errmsg, &len); + if(ret == 0 && len > 0)

space needed between if and ()

+                       strncpy((char *)iov_errmsg->iov_base, errmsg,
+                           iov_errmsg->iov_len);
+               

Extra empty line above.  Unnecessary cast.

+       }
+       
        if (error)
                vfs_freeopts(optlist);
        return (error);

Hope this helps,
--
Nate
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to