In qmp_guest_set_user_password() we have a part of code that we can reuse
in the future commits. Move this code to a separate function.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 qga/commands-posix.c | 139 ++++++++++++++++++++++---------------------
 1 file changed, 72 insertions(+), 67 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 6169bbf7a0..e7b82aaf37 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2114,20 +2114,78 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
 #endif /* __linux__ */
 
 #if defined(__linux__) || defined(__FreeBSD__)
-void qmp_guest_set_user_password(const char *username,
-                                 const char *password,
-                                 bool crypted,
-                                 Error **errp)
+
+static void run_command(const char *argv[], const char *in_str, Error **errp)
 {
     Error *local_err = NULL;
-    char *passwd_path = NULL;
     pid_t pid;
-    int status;
+    int in_len, status;
     int datafd[2] = { -1, -1 };
+
+    if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
+        error_setg(errp, "cannot create pipe FDs");
+        goto out;
+    }
+
+    pid = fork();
+    if (pid == 0) {
+        close(datafd[1]);
+        setsid();
+        dup2(datafd[0], 0);
+        reopen_fd_to_null(1);
+        reopen_fd_to_null(2);
+
+        execve(argv[0], (char *const *)argv, environ);
+        _exit(EXIT_FAILURE);
+    } else if (pid < 0) {
+        error_setg_errno(errp, errno, "failed to create child process");
+        goto out;
+    }
+    close(datafd[0]);
+    datafd[0] = -1;
+
+    in_len = strlen(in_str);
+
+    if (qemu_write_full(datafd[1], in_str, in_len) != in_len) {
+        error_setg_errno(errp, errno, "cannot write new account password");
+        goto out;
+    }
+    close(datafd[1]);
+    datafd[1] = -1;
+
+    ga_wait_child(pid, &status, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+
+    if (!WIFEXITED(status)) {
+        error_setg(errp, "child process has terminated abnormally");
+        goto out;
+    }
+
+    if (WEXITSTATUS(status)) {
+        error_setg(errp, "child process has failed: %s", argv[0]);
+    }
+
+out:
+    if (datafd[0] != -1) {
+        close(datafd[0]);
+    }
+    if (datafd[1] != -1) {
+        close(datafd[1]);
+    }
+}
+
+void qmp_guest_set_user_password(const char *username,
+                                 const char *password,
+                                 bool crypted,
+                                 Error **errp)
+{
+    char *passwd_path = NULL;
     char *rawpasswddata = NULL;
-    size_t rawpasswdlen;
     char *chpasswddata = NULL;
-    size_t chpasswdlen;
+    size_t rawpasswdlen;
 
     rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp);
     if (!rawpasswddata) {
@@ -2155,79 +2213,26 @@ void qmp_guest_set_user_password(const char *username,
     passwd_path = g_find_program_in_path("chpasswd");
 #endif
 
-    chpasswdlen = strlen(chpasswddata);
-
     if (!passwd_path) {
         error_setg(errp, "cannot find 'passwd' program in PATH");
         goto out;
     }
 
-    if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
-        error_setg(errp, "cannot create pipe FDs");
-        goto out;
-    }
-
-    pid = fork();
-    if (pid == 0) {
-        close(datafd[1]);
-        /* child */
-        setsid();
-        dup2(datafd[0], 0);
-        reopen_fd_to_null(1);
-        reopen_fd_to_null(2);
-
+    const char *argv[] = {
 #ifdef __FreeBSD__
-        const char *h_arg;
-        h_arg = (crypted) ? "-H" : "-h";
-        execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0", NULL);
+        passwd_path, "pw", "usermod", "-n", username,
+        (crypted) ? "-H" : "-h", "0", NULL};
 #else
-        if (crypted) {
-            execl(passwd_path, "chpasswd", "-e", NULL);
-        } else {
-            execl(passwd_path, "chpasswd", NULL);
-        }
+        passwd_path, "chpasswd", (crypted) ? "-e" : NULL, NULL
 #endif
-        _exit(EXIT_FAILURE);
-    } else if (pid < 0) {
-        error_setg_errno(errp, errno, "failed to create child process");
-        goto out;
-    }
-    close(datafd[0]);
-    datafd[0] = -1;
+    };
 
-    if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != chpasswdlen) {
-        error_setg_errno(errp, errno, "cannot write new account password");
-        goto out;
-    }
-    close(datafd[1]);
-    datafd[1] = -1;
-
-    ga_wait_child(pid, &status, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        goto out;
-    }
-
-    if (!WIFEXITED(status)) {
-        error_setg(errp, "child process has terminated abnormally");
-        goto out;
-    }
-
-    if (WEXITSTATUS(status)) {
-        error_setg(errp, "child process has failed to set user password");
-        goto out;
-    }
+    run_command(argv, chpasswddata, errp);
 
 out:
     g_free(chpasswddata);
     g_free(rawpasswddata);
     g_free(passwd_path);
-    if (datafd[0] != -1) {
-        close(datafd[0]);
-    }
-    if (datafd[1] != -1) {
-        close(datafd[1]);
-    }
 }
 #else /* __linux__ || __FreeBSD__ */
 void qmp_guest_set_user_password(const char *username,
-- 
2.34.1

On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.ivanov@virtuozzo.com> wrote:

> In qmp_guest_set_user_password() we have a part of code that we can reuse
> in the future commits. Move this code to a separate function.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  qga/commands-posix.c | 139 ++++++++++++++++++++++---------------------
>  1 file changed, 72 insertions(+), 67 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 6169bbf7a0..e7b82aaf37 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2114,20 +2114,78 @@ int64_t
> qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>  #endif /* __linux__ */
>
>  #if defined(__linux__) || defined(__FreeBSD__)
> -void qmp_guest_set_user_password(const char *username,
> -                                 const char *password,
> -                                 bool crypted,
> -                                 Error **errp)
> +
> +static void run_command(const char *argv[], const char *in_str, Error
> **errp)
>  {
>      Error *local_err = NULL;
> -    char *passwd_path = NULL;
>      pid_t pid;
> -    int status;
> +    int in_len, status;
>      int datafd[2] = { -1, -1 };
> +
> +    if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
> +        error_setg(errp, "cannot create pipe FDs");
> +        goto out;
> +    }
> +
> +    pid = fork();
> +    if (pid == 0) {
> +        close(datafd[1]);
> +        setsid();
> +        dup2(datafd[0], 0);
> +        reopen_fd_to_null(1);
> +        reopen_fd_to_null(2);
> +
> +        execve(argv[0], (char *const *)argv, environ);
> +        _exit(EXIT_FAILURE);
> +    } else if (pid < 0) {
> +        error_setg_errno(errp, errno, "failed to create child process");
> +        goto out;
> +    }
> +    close(datafd[0]);
> +    datafd[0] = -1;
> +
> +    in_len = strlen(in_str);
> +
> +    if (qemu_write_full(datafd[1], in_str, in_len) != in_len) {
> +        error_setg_errno(errp, errno, "cannot write new account
> password");
>

As this is a generic `run_command` function, please generalize errors too.


> +        goto out;
> +    }
> +    close(datafd[1]);
> +    datafd[1] = -1;
> +
> +    ga_wait_child(pid, &status, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto out;
> +    }
> +
> +    if (!WIFEXITED(status)) {
> +        error_setg(errp, "child process has terminated abnormally");
> +        goto out;
> +    }
> +
> +    if (WEXITSTATUS(status)) {
> +        error_setg(errp, "child process has failed: %s", argv[0]);
> +    }
> +
> +out:
> +    if (datafd[0] != -1) {
> +        close(datafd[0]);
> +    }
> +    if (datafd[1] != -1) {
> +        close(datafd[1]);
> +    }
> +}
> +
> +void qmp_guest_set_user_password(const char *username,
> +                                 const char *password,
> +                                 bool crypted,
> +                                 Error **errp)
> +{
> +    char *passwd_path = NULL;
>      char *rawpasswddata = NULL;
> -    size_t rawpasswdlen;
>      char *chpasswddata = NULL;
> -    size_t chpasswdlen;
> +    size_t rawpasswdlen;
>
>      rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen,
> errp);
>      if (!rawpasswddata) {
> @@ -2155,79 +2213,26 @@ void qmp_guest_set_user_password(const char
> *username,
>      passwd_path = g_find_program_in_path("chpasswd");
>  #endif
>
> -    chpasswdlen = strlen(chpasswddata);
> -
>      if (!passwd_path) {
>          error_setg(errp, "cannot find 'passwd' program in PATH");
>          goto out;
>      }
>
> -    if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) {
> -        error_setg(errp, "cannot create pipe FDs");
> -        goto out;
> -    }
> -
> -    pid = fork();
> -    if (pid == 0) {
> -        close(datafd[1]);
> -        /* child */
> -        setsid();
> -        dup2(datafd[0], 0);
> -        reopen_fd_to_null(1);
> -        reopen_fd_to_null(2);
> -
> +    const char *argv[] = {
>  #ifdef __FreeBSD__
> -        const char *h_arg;
> -        h_arg = (crypted) ? "-H" : "-h";
> -        execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0",
> NULL);
> +        passwd_path, "pw", "usermod", "-n", username,
> +        (crypted) ? "-H" : "-h", "0", NULL};
>  #else
> -        if (crypted) {
> -            execl(passwd_path, "chpasswd", "-e", NULL);
> -        } else {
> -            execl(passwd_path, "chpasswd", NULL);
> -        }
> +        passwd_path, "chpasswd", (crypted) ? "-e" : NULL, NULL
>  #endif
> -        _exit(EXIT_FAILURE);
> -    } else if (pid < 0) {
> -        error_setg_errno(errp, errno, "failed to create child process");
> -        goto out;
> -    }
> -    close(datafd[0]);
> -    datafd[0] = -1;
> +    };
>
> -    if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) !=
> chpasswdlen) {
> -        error_setg_errno(errp, errno, "cannot write new account
> password");
> -        goto out;
> -    }
> -    close(datafd[1]);
> -    datafd[1] = -1;
> -
> -    ga_wait_child(pid, &status, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        goto out;
> -    }
> -
> -    if (!WIFEXITED(status)) {
> -        error_setg(errp, "child process has terminated abnormally");
> -        goto out;
> -    }
> -
> -    if (WEXITSTATUS(status)) {
> -        error_setg(errp, "child process has failed to set user password");
> -        goto out;
> -    }
> +    run_command(argv, chpasswddata, errp);
>
>  out:
>      g_free(chpasswddata);
>      g_free(rawpasswddata);
>      g_free(passwd_path);
> -    if (datafd[0] != -1) {
> -        close(datafd[0]);
> -    }
> -    if (datafd[1] != -1) {
> -        close(datafd[1]);
> -    }
>  }
>  #else /* __linux__ || __FreeBSD__ */
>  void qmp_guest_set_user_password(const char *username,
> --
> 2.34.1
>
>