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

[Erik Skultety]

On 06/10/16 09:57, Michal Privoznik wrote:
> On 05.10.2016 09:26, 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.
> 
> Well, this area has always been a mess. And I think it is mostly because
> of how readline API works. The completer is called multiple times and we
> have to do a lot in it in order to provide all the completions.
> Having said that, I'm not sure what can be done here to improve the code
> apart from rewriting it completely from scratch (or switching to a
> different library, e.g. libedit).
> 

Well, depending on which parts exactly you meant need rewriting (if of
course you didn't refer to the whole virsh code then all of the
following is just ambiguous) because even if you tried to rewrite just
parts of it, there's imho very little chance you'd end up with a less
mess that it is now and that is because there are parts of the code that
would prevent you from doing so like the fact that we allow multiple
commands delimited by ';' in the interactive shell. I've looked into
that at least 3 times already and still think that the biggest mess is
the parser which we should start from. It is very unfortunate that we
allow multiple commands simultaneously, since that is preventing us from
using an external library to do the parsing for us in a transparent way.
The code would look sooo much cooler then...sigh...

Erik

> Michal
> 
> --
> 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: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161006/66db9755/attachment-0001.sig>