ctdb-cluster-mutex: Separate out command and file handling

This code is difficult to read and there really is no common code
between the 2 cases.  For example, there is no need to split a
filename into words.  Separating each of the 2 cases into its own
function makes the logic much easier to understand.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13800

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
Autobuild-Date(master): Mon Feb 25 03:40:16 CET 2019 on sn-devel-144

(cherry picked from commit c93430fe8fe530a55b9a04cf6cc660c3d420e333)
(cherry picked from commit d5131afc533102ed5adfb147bf1a316e51810729)

diff --git a/ctdb/server/ctdb_cluster_mutex.c b/ctdb/server/ctdb_cluster_mutex.c
index 330d5fd1d909c2275eba5af7c857dbf3ff911463..2e3cb8112ad6cea6b22a356c59c8fe9a6c976552 100644 (file)
--- a/ctdb/server/ctdb_cluster_mutex.c
+++ b/ctdb/server/ctdb_cluster_mutex.c
@@ -118,72 +118,101 @@ static void cluster_mutex_handler(struct tevent_context *ev,
 
 static char cluster_mutex_helper[PATH_MAX+1] = "";
 
-static bool cluster_mutex_helper_args(TALLOC_CTX *mem_ctx,
-                                     const char *argstring, char ***argv)
+static bool cluster_mutex_helper_args_file(TALLOC_CTX *mem_ctx,
+                                          const char *argstring,
+                                          char ***argv)
 {
-       int nargs, i, ret, n;
-       bool is_command = false;
+       bool ok;
        char **args = NULL;
-       char *strv = NULL;
-       char *t = NULL;
 
-       if (argstring != NULL && argstring[0] == '!') {
-               /* This is actually a full command */
-               is_command = true;
-               t = discard_const(&argstring[1]);
-       } else {
-               is_command = false;
-               t = discard_const(argstring);
+       ok = ctdb_set_helper("cluster mutex helper",
+                            cluster_mutex_helper,
+                            sizeof(cluster_mutex_helper),
+                            "CTDB_CLUSTER_MUTEX_HELPER",
+                            CTDB_HELPER_BINDIR,
+                            "ctdb_mutex_fcntl_helper");
+       if (! ok) {
+               DBG_ERR("ctdb exiting with error: "
+                       "Unable to set cluster mutex helper\n");
+               exit(1);
        }
 
-       ret = strv_split(mem_ctx, &strv, t, " \t");
-       if (ret != 0) {
-               DEBUG(DEBUG_ERR,
-                     ("Unable to parse mutex helper string \"%s\" (%s)\n",
-                      argstring, strerror(ret)));
+
+       /* Array includes default helper, file and NULL */
+       args = talloc_array(mem_ctx, char *, 3);
+       if (args == NULL) {
+               DBG_ERR("Memory allocation error\n");
                return false;
        }
-       n = strv_count(strv);
 
-       args = talloc_array(mem_ctx, char *, n + (is_command ? 1 : 2));
+       args[0] = cluster_mutex_helper;
 
-       if (args == NULL) {
-               DEBUG(DEBUG_ERR,(__location__ " out of memory\n"));
+       args[1] = talloc_strdup(args, argstring);
+       if (args[1] == NULL) {
+               DBG_ERR("Memory allocation error\n");
                return false;
        }
 
-       nargs = 0;
-
-       if (! is_command) {
-               if (!ctdb_set_helper("cluster mutex helper",
-                                    cluster_mutex_helper,
-                                    sizeof(cluster_mutex_helper),
-                                    "CTDB_CLUSTER_MUTEX_HELPER",
-                                    CTDB_HELPER_BINDIR,
-                                    "ctdb_mutex_fcntl_helper")) {
-                       DEBUG(DEBUG_ERR,("ctdb exiting with error: %s\n",
-                                        __location__
-                                        " Unable to set cluster mutex helper\n"));
-                       exit(1);
-               }
+       args[2] = NULL;
+
+       *argv = args;
+       return true;
+}
 
-               args[nargs++] = cluster_mutex_helper;
+static bool cluster_mutex_helper_args_cmd(TALLOC_CTX *mem_ctx,
+                                         const char *argstring,
+                                         char ***argv)
+{
+       int i, ret, n;
+       char **args = NULL;
+       char *strv = NULL;
+       char *t = NULL;
+
+       ret = strv_split(mem_ctx, &strv, argstring, " \t");
+       if (ret != 0) {
+               D_ERR("Unable to parse mutex helper command \"%s\" (%s)\n",
+                     argstring,
+                     strerror(ret));
+               return false;
        }
+       n = strv_count(strv);
+
+       /* Extra slot for NULL */
+       args = talloc_array(mem_ctx, char *, n + 1);
+       if (args == NULL) {
+               DBG_ERR("Memory allocation error\n");
+               return false;
+       }
+
+       talloc_steal(args, strv);
 
        t = NULL;
-       for (i = 0; i < n; i++) {
-               /* Don't copy, just keep cmd_args around */
+       for (i = 0 ; i < n; i++) {
                t = strv_next(strv, t);
-               args[nargs++] = t;
+               args[i] = t;
        }
 
-       /* Make sure last argument is NULL */
-       args[nargs] = NULL;
+       args[n] = NULL;
 
        *argv = args;
        return true;
 }
 
+static bool cluster_mutex_helper_args(TALLOC_CTX *mem_ctx,
+                                     const char *argstring,
+                                     char ***argv)
+{
+       bool ok;
+
+       if (argstring != NULL && argstring[0] == '!') {
+               ok = cluster_mutex_helper_args_cmd(mem_ctx, &argstring[1], argv);
+       } else {
+               ok = cluster_mutex_helper_args_file(mem_ctx, argstring, argv);
+       }
+
+       return ok;
+}
+
 struct ctdb_cluster_mutex_handle *
 ctdb_cluster_mutex(TALLOC_CTX *mem_ctx,
                   struct ctdb_context *ctdb,