[libvirt] [PATCH] vsh: Fix warnings in command line completer

Martin Kletzander mkletzan at redhat.com
Wed Oct 5 14:07:09 UTC 2016


On Wed, Oct 05, 2016 at 09:26:43AM +0200, Jiri Denemark wrote:
>GCC complained that
>
>vsh.c: In function 'vshReadlineOptionsGenerator':
>vsh.c:2622:29: warning: unused variable 'opt' [-Wunused-variable]
>         const vshCmdOptDef *opt = &cmd->opts[list_index];
>                             ^
>vsh.c: In function 'vshReadlineParse':
>vsh.c:2830:44: warning: 'opt' may be used uninitialized in this function
>[-Wmaybe-uninitialized]
>             completed_list = opt->completer(autoCompleteOpaque,
>
>Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
>---
>
>Notes:
>    I'm not very fond of the second hunk, but the completer is such
>    a horrible piece of code I couldn't believe it was pushed. I just
>    made the easiest fix and ran away from the code screaming.
>
> tools/vsh.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/tools/vsh.c b/tools/vsh.c
>index cdd1cba..420eb79 100644
>--- a/tools/vsh.c
>+++ b/tools/vsh.c
>@@ -2619,7 +2619,6 @@ vshReadlineOptionsGenerator(const char *text, int state, const vshCmdDef *cmd_pa
>         return NULL;
>
>     while ((name = cmd->opts[list_index].name)) {
>-        const vshCmdOptDef *opt = &cmd->opts[list_index];
>         char *res;
>
>         list_index++;
>@@ -2648,7 +2647,7 @@ vshReadlineParse(const char *text, int state)
>     static vshCommandParser parser, sanitizer;
>     vshCommandToken tk;
>     static const vshCmdDef *cmd;
>-    const vshCmdOptDef *opt;
>+    const vshCmdOptDef *opt = NULL;

Always a good thing to do, you can't broke something that worked by
initialization.  And it's nicer.  Even though Peter^Wsome people would
argue that it's pointless, especially for ancient and weird compilers
(i.e. even slightly different compiler version that they're running or
with one different setting).  But looking at the function it's easy to
dereference that opt lines below in that function.  Same way cmd_exists
can be used without initialization and I believe half of those hundred
variables too.  So the thing you're fixing with this hunk still needs a
proper fix, and I don't know whether we want to hide that.  But looking
at the code, there's already so much stuff hidden...

Basically, I'm trying to say "ACK, you were right, I'm running away too
now!", which translates to "ACK" in tl;dr language.

>     char *tkdata, *optstr, *const_tkdata, *completed_name;
>     char *res = NULL;
>     static char *ctext, *sanitized_text;
>--
>2.10.0
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161005/b46a0beb/attachment-0001.sig>


More information about the libvir-list mailing list