[libvirt] [PATCH 02/11] vshCommandOpt: Relax check for valid options

[Martin Kletzander]

On Tue, Nov 07, 2017 at 01:22:50PM +0100, Michal Privoznik wrote:
>When trying to get an opt for command typed on the command line
>we first check if command has such option. Because if it doesn't
>it is a programming error. For instance: vshCommandOptBool(cmd,
>"config") called from say cmdStart() doesn't make sense since
>there's no --config for the start command. However, we will want
>to have generic completers which are going to check if various
>options are set. And so it can happen that we will check for
>non-existent option for given command. Therefore, we need to
>relax the check otherwise we will hit the assert() and don't get
>anywhere.
>

I would prefer keeping the assert there since it's such an easy check
for that kind of programming error.  Is there no way to distinguish
between calls from the completer?  If not, then I would rename it to
vshCommandOptInternal(), add a bool argument, make vshCommandOpt() call
it with one value and the completer with another one (I don't care if
there's yet another function for that or if completers use the Internal
one).

>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> tools/vsh.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/tools/vsh.c b/tools/vsh.c
>index eca312b4b..24ea45aa4 100644
>--- a/tools/vsh.c
>+++ b/tools/vsh.c
>@@ -815,8 +815,7 @@ vshCommandFree(vshCmd *cmd)
>  * Look up an option passed to CMD by NAME.  Returns 1 with *OPT set
>  * to the option if found, 0 with *OPT set to NULL if the name is
>  * valid and the option is not required, -1 with *OPT set to NULL if
>- * the option is required but not present, and assert if NAME is not
>- * valid (which indicates a programming error).  No error messages are
>+ * the option is required but not present. No error messages are
>  * issued if a value is returned.
>  */
> static int
>@@ -829,8 +828,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>
>     /* See if option is valid and/or required.  */
>     *opt = NULL;
>-    while (valid) {
>-        assert(valid->name);
>+    while (valid && valid->name) {
>         if (STREQ(name, valid->name))
>             break;
>         valid++;
>-- 
>2.13.6
>
>--
>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: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171108/4b98f035/attachment-0001.sig>