3:49 a.m.
On 10/31/13 01:18, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
commit 2b172a8effa712aee97a21a64d2d02060958f9b2 allow
alias to expand to opt=value pair.
That means alias may not look alike since then.
With this patch we will also track alias.
If we type command with one option and another marked
as its alias, we will get an error message like:
error: option '--AA' duplicates its alias '--AAA'
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
v2: do not depend on orders of options in array.
Alias could be anywhere except after the default one.
tools/virsh.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c
index bad78c9..ffead84 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1071,6 +1071,7 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const
char *name,
size_t i;
const vshCmdOptDef *ret = NULL;
char *alias = NULL;
+ char *tmp_name = NULL;
if (STREQ(name, helpopt.name)) {
return &helpopt;
@@ -1101,6 +1102,13 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const
char *name,
if (VIR_STRDUP(*optstr, value + 1) < 0)
goto cleanup;
}
+ tmp_name = vshMalloc(NULL, strlen(name) + 3);
"ctl" is passed to this function so you can pass it along to vshMalloc
+ snprintf(tmp_name, strlen(name) + 3,
"--%s", name);
and you can completely avoid these two lines above by using virAsprintf.
Also this whole code is in a loop and you are allocating the buffer at
EVERY iteration and not freeing it afterwards. This would leak tons of memory.
+ if (strstr(rl_line_buffer, tmp_name)) {
rl_line_buffer can possibly be NULL here which makes virsh crash here.
(see test failure below)
+ vshError(ctl, _("option '--%s'
duplicates its alias '--%s'"),
+ cmd->opts[i].name, name);
+ goto cleanup;
+ }
continue;
}
if ((*opts_seen & (1 << i)) && opt->type !=
VSH_OT_ARGV) {
@@ -1119,6 +1127,7 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const
char *name,
cmd->name, name);
}
cleanup:
+ VIR_FREE(tmp_name);
VIR_FREE(alias);
return ret;
}
This patch also makes the testsuite fail with:
~/libvirt/tests $ VIR_TEST_DEBUG=2 VIR_TEST_RANGE=50-55 ./virshtest
TEST: virshtest
50) virsh echo 33 ... libvirt: error : internal error: Child process (3746826)
unexpected fatal signal 11
FAILED
51) virsh echo 34 ... libvirt: error : internal error: Child process (3746843)
unexpected fatal signal 11
FAILED
52) virsh echo 35 ... libvirt: error : internal error: Child process (3746857)
unexpected fatal signal 11
FAILED
Please run the testsuite before posting patches. You are wasting
review time otherwise.
Peter
P.S: gdb session tracing the problem:
(gdb) run echo --str hellp
Starting program: /home/pipo/libvirt/tools/.libs/virsh echo --str hellp
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need "set solib-search-path" or "set sysroot"?
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff33bfdf5 in __strstr_sse42 () from /lib64/libc.so.6
(gdb) t a a bt
Thread 1 (Thread 0x7ffff7faf840 (LWP 3746903)):
#0 0x00007ffff33bfdf5 in __strstr_sse42 () from /lib64/libc.so.6
#1 0x0000555555576577 in vshCmddefGetOption (ctl=0x7fffffffd980, cmd=0x5555557c9230
<virshCmds+80>, name=0x55555581c460 "string",
opts_seen=0x7fffffffd6bc, opt_index=0x7fffffffd6c0, optstr=0x7fffffffd6d0) at
virsh.c:1107
#2 0x0000555555578295 in vshCommandParse (ctl=0x7fffffffd980, parser=0x7fffffffd760) at
virsh.c:1897
#3 0x0000555555578944 in vshCommandArgvParse (ctl=0x7fffffffd980, nargs=3,
argv=0x7fffffffdb60) at virsh.c:2056
#4 0x000055555557b91b in vshParseArgv (ctl=0x7fffffffd980, argc=4, argv=0x7fffffffdb58)
at virsh.c:3236
#5 0x000055555557bbfb in main (argc=4, argv=0x7fffffffdb58) at virsh.c:3361
(gdb) up
#1 0x0000555555576577 in vshCmddefGetOption (ctl=0x7fffffffd980, cmd=0x5555557c9230
<virshCmds+80>, name=0x55555581c460 "string",
opts_seen=0x7fffffffd6bc, opt_index=0x7fffffffd6c0, optstr=0x7fffffffd6d0) at
virsh.c:1107
1107 if (strstr(rl_line_buffer, tmp_name)) {
(gdb) p rl_line_buffer
$1 = 0x0