glusterd-volgen.c: fix setting the key export-dir

The return value of gf_asprintf() was used as the key length,
and it was stored into ret.
ret was re-used before the dict function was called, therefore was invalid
as the real key length and contained a different value.

That was masked by the fact the key length was only used in key hash, so
while it was incorrect, it was harmless. The hash was consistent and
the key length was re-calculated anyway in dict_set_lk().

This patch fixes it, so later on we can use the key length also in
dict_set_lk() to save another strlen() (sent in a different patch).

However, in the course of this patch I've also decided to reduce the
copy-pasta in this code path and put all NFS options in an array
and iterate through them. This makes the code shorter and easier to read.
(It's also more efficient since once an entry was found, there's no
additional strcmp()'s as the previous code did).

Change-Id: I968ed50a55f3b1a7ad027c72b06e0fa3788eaa9b
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

2 files changed, 41 insertions, 108 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 8243548f881..a1e040a67de 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -4464,152 +4464,79 @@ static int
 nfs_option_handler(volgen_graph_t *graph, struct volopt_map_entry *vme,
                    void *param)
 {
+    static struct nfs_opt nfs_opts[] = {
+        /* {pattern, printf_pattern} */
+        {"!rpc-auth.addr.*.allow", "rpc-auth.addr.%s.allow"},
+        {"!rpc-auth.addr.*.reject", "rpc-auth.addr.%s.reject"},
+        {"!rpc-auth.auth-unix.*", "rpc-auth.auth-unix.%s"},
+        {"!rpc-auth.auth-null.*", "rpc-auth.auth-null.%s"},
+        {"!nfs3.*.trusted-sync", "nfs3.%s.trusted-sync"},
+        {"!nfs3.*.trusted-write", "nfs3.%s.trusted-write"},
+        {"!nfs3.*.volume-access", "nfs3.%s.volume-access"},
+        {"!rpc-auth.ports.*.insecure", "rpc-auth.ports.%s.insecure"},
+        {"!nfs-disable", "nfs.%s.disable"},
+        {NULL, NULL}};
     xlator_t *xl = NULL;
     char *aa = NULL;
     int ret = 0;
     glusterd_volinfo_t *volinfo = NULL;
+    int keylen;
+    struct nfs_opt *opt = NULL;
 
     volinfo = param;
 
-    xl = first_of(graph);
-
     if (!volinfo || (volinfo->volname[0] == '\0'))
         return 0;
 
     if (!vme || !(vme->option))
         return 0;
 
-    if (!strcmp(vme->option, "!rpc-auth.addr.*.allow")) {
-        ret = gf_asprintf(&aa, "rpc-auth.addr.%s.allow", volinfo->volname);
-
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
-        }
-
-        if (ret)
-            return -1;
-    }
-
-    if (!strcmp(vme->option, "!rpc-auth.addr.*.reject")) {
-        ret = gf_asprintf(&aa, "rpc-auth.addr.%s.reject", volinfo->volname);
-
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
-        }
-
-        if (ret)
-            return -1;
-    }
-
-    if (!strcmp(vme->option, "!rpc-auth.auth-unix.*")) {
-        ret = gf_asprintf(&aa, "rpc-auth.auth-unix.%s", volinfo->volname);
-
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
-        }
-
-        if (ret)
-            return -1;
-    }
-    if (!strcmp(vme->option, "!rpc-auth.auth-null.*")) {
-        ret = gf_asprintf(&aa, "rpc-auth.auth-null.%s", volinfo->volname);
-
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
-        }
-
-        if (ret)
-            return -1;
-    }
+    xl = first_of(graph);
 
-    if (!strcmp(vme->option, "!nfs3.*.trusted-sync")) {
-        ret = gf_asprintf(&aa, "nfs3.%s.trusted-sync", volinfo->volname);
+    for (opt = nfs_opts; opt->pattern; opt++) {
+        if (!strcmp(vme->option, opt->pattern)) {
+            keylen = gf_asprintf(&aa, opt->printf_pattern, volinfo->volname);
 
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
-        }
-
-        if (ret)
-            return -1;
-    }
-
-    if (!strcmp(vme->option, "!nfs3.*.trusted-write")) {
-        ret = gf_asprintf(&aa, "nfs3.%s.trusted-write", volinfo->volname);
+            if (keylen == -1) {
+                return -1;
+            }
 
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
+            ret = xlator_set_option(xl, aa, keylen, vme->value);
             GF_FREE(aa);
-        }
 
-        if (ret)
-            return -1;
-    }
-
-    if (!strcmp(vme->option, "!nfs3.*.volume-access")) {
-        ret = gf_asprintf(&aa, "nfs3.%s.volume-access", volinfo->volname);
+            if (ret)
+                return -1;
 
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
+            goto out;
         }
-
-        if (ret)
-            return -1;
     }
 
     if (!strcmp(vme->option, "!nfs3.*.export-dir")) {
-        ret = gf_asprintf(&aa, "nfs3.%s.export-dir", volinfo->volname);
-
-        if (ret != -1) {
-            ret = gf_canonicalize_path(vme->value);
-            if (ret) {
-                GF_FREE(aa);
-                return -1;
-            }
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
-        }
+        keylen = gf_asprintf(&aa, "nfs3.%s.export-dir", volinfo->volname);
 
-        if (ret)
+        if (keylen == -1) {
             return -1;
-    }
-
-    if (!strcmp(vme->option, "!rpc-auth.ports.*.insecure")) {
-        ret = gf_asprintf(&aa, "rpc-auth.ports.%s.insecure", volinfo->volname);
-
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
-            GF_FREE(aa);
         }
 
-        if (ret)
-            return -1;
-    }
-
-    if (!strcmp(vme->option, "!nfs-disable")) {
-        ret = gf_asprintf(&aa, "nfs.%s.disable", volinfo->volname);
-
-        if (ret != -1) {
-            ret = xlator_set_option(xl, aa, ret, vme->value);
+        ret = gf_canonicalize_path(vme->value);
+        if (ret) {
             GF_FREE(aa);
+            return -1;
         }
+        ret = xlator_set_option(xl, aa, keylen, vme->value);
+        GF_FREE(aa);
 
         if (ret)
             return -1;
-    }
-
-    if ((strcmp(vme->voltype, "nfs/server") == 0) && (vme->option[0] != '!')) {
+    } else if ((strcmp(vme->voltype, "nfs/server") == 0) &&
+               (vme->option[0] != '!')) {
         ret = xlator_set_option(xl, vme->option, strlen(vme->option),
                                 vme->value);
         if (ret)
             return -1;
     }
 
+out:
     return 0;
 }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.h b/xlators/mgmt/glusterd/src/glusterd-volgen.h
index 5c4bfe0db0b..406fcf52817 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.h
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.h
@@ -174,6 +174,12 @@ struct volgen_brick_xlator {
      * delay-gen before this xlator */
     char *dbg_key;
 };
+
+struct nfs_opt {
+    const char *pattern;
+    const char *printf_pattern;
+};
+
 typedef struct volgen_brick_xlator volgen_brick_xlator_t;
 
 int