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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 11, 2018 at 08:09:25AM -0500, John Ferlan wrote:


On 01/11/2018 07:55 AM, Martin Kletzander wrote:
On Thu, Jan 11, 2018 at 06:02:58AM -0500, John Ferlan wrote:


On 01/11/2018 05:50 AM, Martin Kletzander wrote:
On Tue, Jan 02, 2018 at 06:12:01PM +0100, 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@xxxxxxxxxx>
---
tools/vsh.c | 7 ++++---
tools/vsh.h | 3 ++-
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index ebc8d9cb1..d27acb95b 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,
@@ -829,7 +829,8 @@ 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 (!cmd->skipChecks)
+            assert(valid->name);

This can segfault when cmd->skipChecks == False && valid->name == NULL,
which is what the assert() guarded before.

So either STREQ_NULLABLE or another if.


Hmmm... Also see:

https://www.redhat.com/archives/libvir-list/2017-December/msg00605.html

it's related somewhat...


I don't see how, this is all wrapped in `while (valid)`

The other patch is "after" the loop.

Look at the entire context... although we know it's a software
engineering error to not have some sort of match, some compiler believes
we can exit the "while (valid)" loop with "valid == NULL", follwed by
the next assert which dereferences @valid without asserting if valid is
non-NULL.


Oh, I guess that patch from Marc should be pushed (I still didn't get to
soooo many patches on the list) and this should then handle addition to
that change as well.  I got confused by your vague "somewhat related" =)

I didn't say it was exactly related, just "related somewhat".

John

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

Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux