[libvirt] [PATCH 08/18 v2] vshCommandOpt: Allow caller avoiding assert()

Michal Privoznik posted 18 patches 7 years, 4 months ago
[libvirt] [PATCH 08/18 v2] vshCommandOpt: Allow caller avoiding assert()
Posted by Michal Privoznik 7 years, 4 months ago
In the future, completer callbacks will receive partially parsed
command (and thus possibly incomplete). However, we still want
them to use command options fetching APIs we already have (e.g.
vshCommandOpt*()) and at the same time don't report any errors
(nor call any asserts).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/vsh.c | 24 ++++++++++++++----------
 tools/vsh.h |  3 ++-
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 54e59fff6..b113c8c95 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
  * 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
- * issued if a value is returned.
+ * valid (which indicates a programming error) unless cmd->skipChecks
+ * is set. No error messages are issued if a value is returned.
  */
 static int
 vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
@@ -828,15 +828,19 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
 
     /* See if option is valid and/or required.  */
     *opt = NULL;
-    while (valid) {
-        assert(valid->name);
-        if (STREQ(name, valid->name))
-            break;
-        valid++;
+
+    if (!cmd->skipChecks) {
+        while (valid && valid->name) {
+            if (STREQ(name, valid->name))
+                break;
+            valid++;
+        }
+
+        assert(valid && (!needData || valid->type != VSH_OT_BOOL));
+
+        if (valid->flags & VSH_OFLAG_REQ)
+            ret = -1;
     }
-    assert(!needData || valid->type != VSH_OT_BOOL);
-    if (valid->flags & VSH_OFLAG_REQ)
-        ret = -1;
 
     /* See if option is present on command line.  */
     while (candidate) {
diff --git a/tools/vsh.h b/tools/vsh.h
index 8f7df9ff8..112b1b57d 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -188,7 +188,8 @@ struct _vshCmdDef {
 struct _vshCmd {
     const vshCmdDef *def;       /* command definition */
     vshCmdOpt *opts;            /* list of command arguments */
-    vshCmd *next;      /* next command */
+    vshCmd *next;               /* next command */
+    bool skipChecks;            /* skip validity checks when retrieving opts */
 };
 
 /*
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/18 v2] vshCommandOpt: Allow caller avoiding assert()
Posted by John Ferlan 7 years, 4 months ago

On 01/11/2018 11:38 AM, Michal Privoznik wrote:
> In the future, completer callbacks will receive partially parsed
> command (and thus possibly incomplete). However, we still want
> them to use command options fetching APIs we already have (e.g.
> vshCommandOpt*()) and at the same time don't report any errors
> (nor call any asserts).
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tools/vsh.c | 24 ++++++++++++++----------
>  tools/vsh.h |  3 ++-
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 

Looks good to me...

Reviewed-by: John Ferlan <jferlan@redhat.com>

Tks-

John

> diff --git a/tools/vsh.c b/tools/vsh.c
> index 54e59fff6..b113c8c95 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -815,8 +815,8 @@ vshCommandFree(vshCmd *cmd)
>   * 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
> - * issued if a value is returned.
> + * valid (which indicates a programming error) unless cmd->skipChecks
> + * is set. No error messages are issued if a value is returned.
>   */
>  static int
>  vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
> @@ -828,15 +828,19 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
>  
>      /* See if option is valid and/or required.  */
>      *opt = NULL;
> -    while (valid) {
> -        assert(valid->name);
> -        if (STREQ(name, valid->name))
> -            break;
> -        valid++;
> +
> +    if (!cmd->skipChecks) {
> +        while (valid && valid->name) {
> +            if (STREQ(name, valid->name))
> +                break;
> +            valid++;
> +        }
> +
> +        assert(valid && (!needData || valid->type != VSH_OT_BOOL));
> +
> +        if (valid->flags & VSH_OFLAG_REQ)
> +            ret = -1;
>      }
> -    assert(!needData || valid->type != VSH_OT_BOOL);
> -    if (valid->flags & VSH_OFLAG_REQ)
> -        ret = -1;
>  
>      /* See if option is present on command line.  */
>      while (candidate) {
> diff --git a/tools/vsh.h b/tools/vsh.h
> index 8f7df9ff8..112b1b57d 100644
> --- a/tools/vsh.h
> +++ b/tools/vsh.h
> @@ -188,7 +188,8 @@ struct _vshCmdDef {
>  struct _vshCmd {
>      const vshCmdDef *def;       /* command definition */
>      vshCmdOpt *opts;            /* list of command arguments */
> -    vshCmd *next;      /* next command */
> +    vshCmd *next;               /* next command */
> +    bool skipChecks;            /* skip validity checks when retrieving opts */
>  };
>  
>  /*
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list