text-based mount command: fix return value from po_rightmost()

Recently commit 0dcb83a8 changed the po_rightmost() function to
distinguish among several possible mount options by taking a table
containing the alternatives, and returning the table index of the
entry which is rightmost in the mount option string.

If it didn't find any mount option that matches an entry from the
passed-in table, it returned zero.  This was the same behavior it had
before, when it only checked for two options at a time.  It returned
PO_NEITHER_FOUND, which was zero.

Since this is C, however, zero also happens to be a valid index into
the passed-in array of options.

Modify the po_rightmost() function to return -1 if the entry wasn't
found, and fix up the callers to look for a C-style array index that
starts at zero.

Thanks to Steve Dickson for troubleshooting the problem.  His solution
was merely to bump the return value, as callers already expected an
ordinal index instead of a C-style index.

I prefer this equivalent but slightly more extensive change because it
makes the behavior of po_rightmost() more closely match how humans
understand C arrays to work.  Let's address some of the confusion that
caused this bug, as well as fixing the run-time behavior.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 91a005cc9fc5cac052a5876d79dc8d041048ecf5..a97495392274edc0aa4a1021b70138c3dbb83f13 100644 (file)
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1168,16 +1168,16 @@ static rpcvers_t nfs_nfs_version(struct mount_options *options)
        long tmp;
 
        switch (po_rightmost(options, nfs_version_opttbl)) {
-       case 1: /* v2 */
+       case 0: /* v2 */
                return 2;
-       case 2: /* v3 */
+       case 1: /* v3 */
                return 3;
-       case 3: /* vers */
+       case 2: /* vers */
                if (po_get_numeric(options, "vers", &tmp) == PO_FOUND)
                        if (tmp >= 2 && tmp <= 3)
                                return tmp;
                break;
-       case 4: /* nfsvers */
+       case 3: /* nfsvers */
                if (po_get_numeric(options, "nfsvers", &tmp) == PO_FOUND)
                        if (tmp >= 2 && tmp <= 3)
                                return tmp;
@@ -1198,11 +1198,9 @@ static unsigned short nfs_nfs_protocol(struct mount_options *options)
        char *option;
 
        switch (po_rightmost(options, nfs_transport_opttbl)) {
-       case 1: /* udp */
-               return IPPROTO_UDP;
-       case 2: /* tcp */
+       case 1: /* tcp */
                return IPPROTO_TCP;
-       case 3: /* proto */
+       case 2: /* proto */
                option = po_get(options, "proto");
                if (option) {
                        if (strcmp(option, "tcp") == 0)
@@ -1211,6 +1209,7 @@ static unsigned short nfs_nfs_protocol(struct mount_options *options)
                                return IPPROTO_UDP;
                }
        }
+
        return IPPROTO_UDP;
 }
 

diff --git a/utils/mount/parse_opt.c b/utils/mount/parse_opt.c
index 493450859f2235714caaca5391496f8eed85a588..1dfee8aafe1b97bad1582361becc64d8dc69eb51 100644 (file)
--- a/utils/mount/parse_opt.c
+++ b/utils/mount/parse_opt.c
@@ -437,9 +437,10 @@ po_found_t po_get_numeric(struct mount_options *options, char *keyword, long *va
  * as "proto=," "udp" and "tcp."
  *
  * Returns the index into @keys of the option that is rightmost.
- * If none of the options are present, returns zero.
+ * If none of the options listed in @keys is present in @options, or
+ * if @options is NULL, returns -1.
  */
-unsigned int po_rightmost(struct mount_options *options, const char *keys[])
+int po_rightmost(struct mount_options *options, const char *keys[])
 {
        struct mount_option *option;
        unsigned int i;
@@ -452,7 +453,7 @@ unsigned int po_rightmost(struct mount_options *options, const char *keys[])
                }
        }
 
-       return 0;
+       return -1;
 }
 
 /**

diff --git a/utils/mount/parse_opt.h b/utils/mount/parse_opt.h
index e132b1cd979961cb534608d5721cbf38fa21781a..f9243c355cbddcebde3938f78f7c041033519095 100644 (file)
--- a/utils/mount/parse_opt.h
+++ b/utils/mount/parse_opt.h
@@ -47,7 +47,7 @@ po_found_t            po_contains(struct mount_options *, char *);
 char *                 po_get(struct mount_options *, char *);
 po_found_t             po_get_numeric(struct mount_options *,
                                        char *, long *);
-unsigned int           po_rightmost(struct mount_options *,
+int                    po_rightmost(struct mount_options *,
                                        const char *keys[]);
 po_found_t             po_remove_all(struct mount_options *, char *);
 void                   po_destroy(struct mount_options *);

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 6d44bb75791652b81a7b58d1b107a5124fd94747..770b5b515c494d135966497ad27ba7c73ccea7c8 100644 (file)
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -232,7 +232,7 @@ static const char *nfs_lock_opttbl[] = {
 
 static int nfs_verify_lock_option(struct mount_options *options)
 {
-       if (po_rightmost(options, nfs_lock_opttbl) == 1)
+       if (po_rightmost(options, nfs_lock_opttbl) == 0)
                return 1;
 
        if (!start_statd()) {
@@ -756,7 +756,7 @@ static int nfsmount_start(struct nfsmount_info *mi)
        if (!nfs_validate_options(mi))
                return EX_FAIL;
 
-       if (po_rightmost(mi->options, nfs_background_opttbl) == 1)
+       if (po_rightmost(mi->options, nfs_background_opttbl) == 0)
                return nfsmount_bg(mi);
        else
                return nfsmount_fg(mi);