Re: [PATCH v2 05/25] virCommandToString: Allow stripping command path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On a Friday in 2021, Peter Krempa wrote:
In tests we don't want to use the full path to commands as it's
unpleasant to keep that working on all systems.

Add an integrated way to strip the prefix which will be used to replace
virTestClearCommandPath() as a more systemic solution.

Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
src/libvirt_private.syms |  1 +
src/util/vircommand.c    | 35 ++++++++++++++++++++++++++++++++---
src/util/vircommand.h    |  4 ++++
3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f36400b5f6..7dd3a1ee10 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2031,6 +2031,7 @@ virCommandSetUID;
virCommandSetUmask;
virCommandSetWorkingDirectory;
virCommandToString;
+virCommandToStringFull;
virCommandWait;
virCommandWriteArgLog;
virFork;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index eb70f8cf85..8ae5badf0f 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2055,9 +2055,10 @@ virCommandWriteArgLog(virCommandPtr cmd, int logfd)


/**
- * virCommandToString:
+ * virCommandToStringFull:
 * @cmd: the command to convert
 * @linebreaks: true to break line after each env var or option
+ * @stripCommandPath: strip the path leading to the binary of @cmd
 *
 * Call after adding all arguments and environment settings, but
 * before Run/RunAsync, to return a string representation of the
@@ -2067,11 +2068,15 @@ virCommandWriteArgLog(virCommandPtr cmd, int logfd)
 * Caller is responsible for freeing the resulting string.
 */
char *
-virCommandToString(virCommandPtr cmd, bool linebreaks)
+virCommandToStringFull(virCommandPtr cmd,
+                       bool linebreaks,
+                       bool stripCommandPath)
{
    size_t i;
    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
    bool prevopt = false;
+    const char *command = cmd->args[0];
+    g_autofree char *basename = NULL;

    /* Cannot assume virCommandRun will be called; so report the error
     * now.  If virCommandRun is called, it will report the same error. */
@@ -2097,7 +2102,11 @@ virCommandToString(virCommandPtr cmd, bool linebreaks)
        if (linebreaks)
            virBufferAddLit(&buf, "\\\n");
    }
-    virBufferEscapeShell(&buf, cmd->args[0]);
+
+    if (stripCommandPath)
+        command = basename = g_path_get_basename(command);
+
+    virBufferEscapeShell(&buf, command);
    for (i = 1; i < cmd->nargs; i++) {
        virBufferAddChar(&buf, ' ');
        if (linebreaks) {
@@ -2116,6 +2125,26 @@ virCommandToString(virCommandPtr cmd, bool linebreaks)
}


+/**
+ * virCommandToString:
+ * @cmd: the command to convert
+ * @linebreaks: true to break line after each env var or option
+ *
+ * Call after adding all arguments and environment settings, but
+ * before Run/RunAsync, to return a string representation of the
+ * environment and arguments of cmd, suitably quoted for pasting into
+ * a shell.  If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), NULL will be returned.
+ * Caller is responsible for freeing the resulting string.
+ */

Here you copied the orignal comment, including the out-of-date reference
to out-of-memory conditions.

Consider dropping the comment completely since:
a) it's internal API so it does not end up in a generated doc
b) copying comments ensures not just one but two will go out-of-date
eventually
c) anyone needing to know what virCommandToString does can just look at
virCommandToStringFull instead

+char *
+virCommandToString(virCommandPtr cmd,
+                   bool linebreaks)
+{
+    return virCommandToStringFull(cmd, linebreaks, false);
+}
+
+
int
virCommandGetArgList(virCommandPtr cmd,
                     char ***args,

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux