run: fix bad escaping and memory ownership confusion

arg_description was either set to arg_unit (i.e. a const char*), or to
char *description, the result of allocation in run(). But description
was decorated with _cleanup_, so it would be freed when going out of the
function. Nothing bad would happen, because the program would exit after
exiting from run(), but this is just all too messy.

Also, strv_join(" ") + shell_escape() is not a good way to escape command
lines. In particular, one the join has happened, we cannot distinguish
empty arguments, or arguments with whitespace, etc. We have a helper
function to do the escaping properly, so let's use that.

Fixup for 2c29813da3421b77eca5e5cdc3b9a863cad473b9.

diff --git a/src/run/run.c b/src/run/run.c
index 5c0f3ad2474da9d36a3f65c48205f18f0da6c1f9..88eca0fd6d172c1d68bc0473b65c92326ee735b5 100644 (file)
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -43,7 +43,7 @@ static bool arg_remain_after_exit = false;
 static bool arg_no_block = false;
 static bool arg_wait = false;
 static const char *arg_unit = NULL;
-static const char *arg_description = NULL;
+static char *arg_description = NULL;
 static const char *arg_slice = NULL;
 static bool arg_slice_inherit = false;
 static int arg_expand_environment = -1;
@@ -74,6 +74,7 @@ static char *arg_working_directory = NULL;
 static bool arg_shell = false;
 static char **arg_cmdline = NULL;
 
+STATIC_DESTRUCTOR_REGISTER(arg_description, freep);
 STATIC_DESTRUCTOR_REGISTER(arg_environment, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_property, strv_freep);
 STATIC_DESTRUCTOR_REGISTER(arg_path_property, strv_freep);
@@ -281,7 +282,9 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
 
                 case ARG_DESCRIPTION:
-                        arg_description = optarg;
+                        r = free_and_strdup(&arg_description, optarg);
+                        if (r < 0)
+                                return r;
                         break;
 
                 case ARG_SLICE:
@@ -1911,7 +1914,6 @@ static bool shall_make_executable_absolute(void) {
 
 static int run(int argc, char* argv[]) {
         _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
-        _cleanup_free_ char *description = NULL;
         int r;
 
         log_show_color(true);
@@ -1936,19 +1938,16 @@ static int run(int argc, char* argv[]) {
         }
 
         if (!arg_description) {
-                if (strv_isempty(arg_cmdline))
-                        arg_description = arg_unit;
-                else {
-                        _cleanup_free_ char *joined = strv_join(arg_cmdline, " ");
-                        if (!joined)
-                                return log_oom();
+                char *t;
 
-                        description = shell_escape(joined, "\"");
-                        if (!description)
-                                return log_oom();
+                if (strv_isempty(arg_cmdline))
+                        t = strdup(arg_unit);
+                else
+                        t = quote_command_line(arg_cmdline, SHELL_ESCAPE_EMPTY);
+                if (!t)
+                        return log_oom();
 
-                        arg_description = description;
-                }
+                free_and_replace(arg_description, t);
         }
 
         /* For backward compatibility reasons env var expansion is disabled by default for scopes, and