8:01 a.m.
Peter Krempa (6):
virsh: Un-document 'virsh echo'
virshtest: Don't use both '--xml' and '--shell' for 'virsh
echo'
virsh: cmdEcho: Make '--xml' and '--shell' mutually exclusive
virsh: cmdEcho: Rewrite with new buffer helpers
virsh: Add testing for vshStringToArray
vshStringToArray: Rewrite using 'g_strsplit'
docs/manpages/virsh.rst | 17 ------
tests/virshtest.c | 20 +++++--
tools/vsh.c | 115 +++++++++++++++++++---------------------
3 files changed, 69 insertions(+), 83 deletions(-)
--
2.31.1
8:01 a.m.
New subject: [PATCH 1/6] virsh: Un-document 'virsh echo'
Note that it's for internal testing use and remove the manpage entry.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 17 -----------------
tools/vsh.c | 4 ++--
2 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 20936994ce..4075929a76 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -907,23 +907,6 @@ print that all CPU models are accepted for these architectures and
the actual
list of supported CPU models can be checked in the domain capabilities XML.
-echo
-----
-
-**Syntax:**
-
-::
-
- echo [--shell] [--xml] [err...] [arg...]
-
-Echo back each *arg*, separated by space. If *--shell* is
-specified, then the output will be single-quoted where needed, so that
-it is suitable for reuse in a shell context. If *--xml* is
-specified, then the output will be escaped for use in XML.
-If *--err* is specified, prefix ``"error: "`` and output to stderr
-instead of stdout.
-
-
hypervisor-cpu-compare
----------------------
diff --git a/tools/vsh.c b/tools/vsh.c
index f9600bafba..f44db5d56d 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3139,10 +3139,10 @@ const vshCmdOptDef opts_echo[] = {
const vshCmdInfo info_echo[] = {
{.name = "help",
- .data = N_("echo arguments")
+ .data = N_("echo arguments. Used for internal testing.")
},
{.name = "desc",
- .data = N_("Echo back arguments, possibly with quoting.")
+ .data = N_("Echo back arguments, possibly with quoting. Used for internal
testing.")
},
{.name = NULL}
};
--
2.31.1
8:01 a.m.
New subject: [PATCH 2/6] virshtest: Don't use both '--xml' and '--shell' for 'virsh echo'
Escaping for both shell and XML makes no sense. Use one at time so that
we can forbid use of both.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/virshtest.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 53db2aa19a..07c27428ae 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -432,9 +432,8 @@ mymain(void)
DO_TEST(15, "a A 0 + * ; . ' " / ? = \n < >
&\n",
"echo", "--xml", "a", "A",
"0", "+", "*", ";", ".",
"'", "\"",
"/", "?", "=", " ", "\n",
"<", ">", "&");
- DO_TEST(16, "a A 0 + '*' ';' . '''
'"' / '?' = ' ' '\n' '<'"
- " '>' '&'\n",
- "echo", "--shell", "--xml", "a",
"A", "0", "+", "*", ";", ".",
"'",
+ DO_TEST(16, "a A 0 + '*' ';' . ''\\'''
'\"' / '?' = ' ' '\n' '<' '>'
'&'\n",
+ "echo", "--shell", "a", "A",
"0", "+", "*", ";", ".",
"\'",
"\"", "/", "?", "=", "
", "\n", "<", ">", "&");
DO_TEST(17, "\n",
"echo", "");
@@ -443,7 +442,7 @@ mymain(void)
DO_TEST(19, "\n",
"echo", "--xml", "");
DO_TEST(20, "''\n",
- "echo", "--xml", "--shell", "");
+ "echo", "--shell", "");
DO_TEST(21, "\n",
"echo ''");
DO_TEST(22, "''\n",
@@ -451,7 +450,7 @@ mymain(void)
DO_TEST(23, "\n",
"echo --xml ''");
DO_TEST(24, "''\n",
- "echo --xml --shell \"\"''");
+ "echo --shell \"\"''");
/* Tests of -- handling. */
DO_TEST(25, "a\n",
--
2.31.1
8:01 a.m.
New subject: [PATCH 3/6] virsh: cmdEcho: Make '--xml' and '--shell' mutually exclusive
Initialize the flags earlier and use VSH_EXCLUSIVE_OPTIONS_VAR to
declare the conflicting options as exclusive.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/vsh.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index f44db5d56d..009c93254c 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3153,20 +3153,15 @@ const vshCmdInfo info_echo[] = {
bool
cmdEcho(vshControl *ctl, const vshCmd *cmd)
{
- bool shell = false;
- bool xml = false;
- bool err = false;
+ bool shell = vshCommandOptBool(cmd, "shell");
+ bool xml = vshCommandOptBool(cmd, "xml");
+ bool err = vshCommandOptBool(cmd, "err");
int count = 0;
const vshCmdOpt *opt = NULL;
g_autofree char *arg = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
- if (vshCommandOptBool(cmd, "shell"))
- shell = true;
- if (vshCommandOptBool(cmd, "xml"))
- xml = true;
- if (vshCommandOptBool(cmd, "err"))
- err = true;
+ VSH_EXCLUSIVE_OPTIONS_VAR(shell, xml);
while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
g_autofree char *str = NULL;
--
2.31.1
8:01 a.m.
New subject: [PATCH 4/6] virsh: cmdEcho: Rewrite with new buffer helpers
Remove the need for temporary strings by fillin the output buffer
directly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/vsh.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 009c93254c..2456267426 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3156,7 +3156,6 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
bool shell = vshCommandOptBool(cmd, "shell");
bool xml = vshCommandOptBool(cmd, "xml");
bool err = vshCommandOptBool(cmd, "err");
- int count = 0;
const vshCmdOpt *opt = NULL;
g_autofree char *arg = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
@@ -3164,27 +3163,21 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
VSH_EXCLUSIVE_OPTIONS_VAR(shell, xml);
while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
- g_autofree char *str = NULL;
- g_auto(virBuffer) xmlbuf = VIR_BUFFER_INITIALIZER;
const char *curr = opt->data;
- if (count)
- virBufferAddChar(&buf, ' ');
-
if (xml) {
- virBufferEscapeString(&xmlbuf, "%s", curr);
- str = virBufferContentAndReset(&xmlbuf);
+ virBufferEscapeString(&buf, "%s", curr);
+ } else if (shell) {
+ virBufferEscapeShell(&buf, curr);
} else {
- str = g_strdup(curr);
+ virBufferAdd(&buf, curr, -1);
}
- if (shell)
- virBufferEscapeShell(&buf, str);
- else
- virBufferAdd(&buf, str, -1);
- count++;
+ virBufferAddChar(&buf, ' ');
}
+ virBufferTrim(&buf, " ");
+
arg = virBufferContentAndReset(&buf);
if (arg) {
if (err)
--
2.31.1
8:01 a.m.
New subject: [PATCH 5/6] virsh: Add testing for vshStringToArray
Add a '--split' switch for the 'virsh echo' command and add few test
cases to the virshtest.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/virshtest.c | 11 +++++++++++
tools/vsh.c | 15 +++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 07c27428ae..751e8ffc49 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -487,6 +487,17 @@ mymain(void)
DO_TEST(46, "a\n", "#unbalanced; 'quotes\"\necho a #
b");
DO_TEST(47, "a\n", "\\# ignored;echo a\n'#also'
ignored");
+ /* test of splitting in vshStringToArray */
+ DO_TEST(48, "a\nb,c,\nd,,e,,\nf,,,e\n",
+ "-q", "echo", "--split",
"a,b,,c,,,d,,,,e,,,,,f,,,,,,e");
+ DO_TEST(49, "\na\nb,c,\nd,,e,,\nf,,,e\n\n",
+ "-q", "echo", "--split",
",a,b,,c,,,d,,,,e,,,,,f,,,,,,e,");
+ DO_TEST(50, ",a\nb,c,\nd,,e,,\nf,,,e,\n",
+ "-q", "echo", "--split",
",,a,b,,c,,,d,,,,e,,,,,f,,,,,,e,,");
+ DO_TEST(51, ",\na\nb,c,\nd,,e,,\nf,,,e,\n\n",
+ "-q", "echo", "--split",
",,,a,b,,c,,,d,,,,e,,,,,f,,,,,,e,,,");
+ DO_TEST(52, ",,a\nb,c,\nd,,e,,\nf,,,e,,\n",
+ "-q", "echo", "--split",
",,,,a,b,,c,,,d,,,,e,,,,,f,,,,,,e,,,,");
# undef DO_TEST
VIR_FREE(custom_uri);
diff --git a/tools/vsh.c b/tools/vsh.c
index 2456267426..cdcf67684e 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3118,6 +3118,10 @@ const vshCmdOptDef opts_echo[] = {
.type = VSH_OT_BOOL,
.help = N_("escape for XML use")
},
+ {.name = "split",
+ .type = VSH_OT_BOOL,
+ .help = N_("split each argument on ','; ',,' is an escape
sequence")
+ },
{.name = "err",
.type = VSH_OT_BOOL,
.help = N_("output to stderr"),
@@ -3156,11 +3160,14 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
bool shell = vshCommandOptBool(cmd, "shell");
bool xml = vshCommandOptBool(cmd, "xml");
bool err = vshCommandOptBool(cmd, "err");
+ bool split = vshCommandOptBool(cmd, "split");
const vshCmdOpt *opt = NULL;
g_autofree char *arg = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
VSH_EXCLUSIVE_OPTIONS_VAR(shell, xml);
+ VSH_EXCLUSIVE_OPTIONS_VAR(shell, split);
+ VSH_EXCLUSIVE_OPTIONS_VAR(xml, split);
while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
const char *curr = opt->data;
@@ -3169,6 +3176,14 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
virBufferEscapeString(&buf, "%s", curr);
} else if (shell) {
virBufferEscapeShell(&buf, curr);
+ } else if (split) {
+ g_auto(GStrv) spl = NULL;
+ GStrv n;
+
+ vshStringToArray(curr, &spl);
+
+ for (n = spl; *n; n++)
+ virBufferAsprintf(&buf, "%s\n", *n);
} else {
virBufferAdd(&buf, curr, -1);
}
--
2.31.1
8:01 a.m.
New subject: [PATCH 6/6] vshStringToArray: Rewrite using 'g_strsplit'
Use 'g_strsplit' to split the strings and then concatenate back when the
escape sequence (',,') is used.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/vsh.c | 62 ++++++++++++++++++++++-------------------------------
1 file changed, 26 insertions(+), 36 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index cdcf67684e..202a507f2e 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -122,48 +122,38 @@ int
vshStringToArray(const char *str,
char ***array)
{
- g_autofree char *str_copied = g_strdup(str);
- char *str_tok = NULL;
- char *tmp;
- unsigned int nstr_tokens = 0;
- char **arr = NULL;
- size_t len = strlen(str_copied);
-
- /* tokenize the string from user and save its parts into an array */
- nstr_tokens = 1;
-
- /* count the delimiters, recognizing ,, as an escape for a
- * literal comma */
- str_tok = str_copied;
- while ((str_tok = strchr(str_tok, ','))) {
- if (str_tok[1] == ',')
- str_tok++;
- else
- nstr_tokens++;
- str_tok++;
- }
+ g_auto(GStrv) tmp = NULL;
+ GStrv n;
+ size_t ntoks = 0;
+ bool concat = false;
+
+ tmp = g_strsplit(str, ",", 0);
+
+ *array = g_new0(char *, g_strv_length(tmp) + 1);
+ (*array)[ntoks++] = g_strdup(tmp[0]);
- /* reserve the NULL element at the end */
- arr = g_new0(char *, nstr_tokens + 1);
+ /* undo splitting of comma escape (',,') by concatenating back on empty
strings */
+ for (n = tmp + 1; n[0]; n++) {
+ if (concat) {
+ g_autofree char *old = (*array)[ntoks - 1];
- /* tokenize the input string, while treating ,, as a literal comma */
- nstr_tokens = 0;
- tmp = str_tok = str_copied;
- while ((tmp = strchr(tmp, ','))) {
- if (tmp[1] == ',') {
- memmove(&tmp[1], &tmp[2], len - (tmp - str_copied) - 2 + 1);
- len--;
- tmp++;
+ (*array)[ntoks - 1] = g_strconcat(old, ",", n[0], NULL);
+ concat = false;
continue;
}
- *tmp++ = '\0';
- arr[nstr_tokens++] = g_strdup(str_tok);
- str_tok = tmp;
+
+ if (strlen(n[0]) == 0) {
+ concat = true;
+ } else {
+ (*array)[ntoks++] = g_strdup(n[0]);
+ }
}
- arr[nstr_tokens++] = g_strdup(str_tok);
- *array = arr;
- return nstr_tokens;
+ /* corner case of ending with a single comma */
+ if (concat)
+ (*array)[ntoks++] = g_strdup("");
+
+ return ntoks;
}
virErrorPtr last_error;
--
2.31.1
9:11 a.m.
On Thu, Aug 12, 2021 at 03:01:30PM +0200, Peter Krempa wrote:
Peter Krempa (6):
virsh: Un-document 'virsh echo'
virshtest: Don't use both '--xml' and '--shell' for 'virsh
echo'
virsh: cmdEcho: Make '--xml' and '--shell' mutually exclusive
virsh: cmdEcho: Rewrite with new buffer helpers
virsh: Add testing for vshStringToArray
vshStringToArray: Rewrite using 'g_strsplit'
With the typo in 4/6 fixed the series is
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
docs/manpages/virsh.rst | 17 ------
tests/virshtest.c | 20 +++++--
tools/vsh.c | 115 +++++++++++++++++++---------------------
3 files changed, 69 insertions(+), 83 deletions(-)
--
2.31.1
9:14 a.m.
On a %A in %Y, Peter Krempa wrote:
Peter Krempa (6):
virsh: Un-document 'virsh echo'
virshtest: Don't use both '--xml' and '--shell' for 'virsh
echo'
virsh: cmdEcho: Make '--xml' and '--shell' mutually exclusive
virsh: cmdEcho: Rewrite with new buffer helpers
virsh: Add testing for vshStringToArray
vshStringToArray: Rewrite using 'g_strsplit'
docs/manpages/virsh.rst | 17 ------
tests/virshtest.c | 20 +++++--
tools/vsh.c | 115 +++++++++++++++++++---------------------
3 files changed, 69 insertions(+), 83 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1198
days inactive
1198
days old
9 comments
3 participants
participants (3)
-
Jano Tomko -
Martin Kletzander -
Peter Krempa