New Coverity issues (tools/libxl/testidl.c)

New Coverity issues (tools/libxl/testidl.c)

[Andrew Cooper]

>From the latest run, there have been two new issues.  They are both in
testidl.c, which appears to be code generated by gentests.py

CID1135378:  in libxl_domain_build_info_rand_init()
CID 1135379:  in libxl_event_rand_init()

In both cases, the complain is that in each function is a
"switch(p->type)" where p->type is completely uninitialised.

The complain looks genuine, but I defer to people more familiar with the
IDL and gentests.py for suggestions of how to fix it.

~Andrew

[PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union

[Ian Campbell]

This is Coverity CID 1135378 and 1135379.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxl/gentest.py |    1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 6fab493..722b7f4 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -42,6 +42,7 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
     elif isinstance(ty, idl.KeyedUnion):
         if parent is None:
             raise Exception("KeyedUnion type must have a parent")
+        s += gen_rand_init(ty.keyvar.type, parent + ty.keyvar.name, indent, parent)
         s += "switch (%s) {\n" % (parent + ty.keyvar.name)
         for f in ty.fields:
             (nparent,fexpr) = ty.member(v, f, parent is None)
-- 
1.7.10.4

Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union

[Andrew Cooper]

On 04/12/13 17:48, Ian Campbell wrote:
> This is Coverity CID 1135378 and 1135379.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

This sound plausible, although given my unfamiliarity with gentest.py, I
dont feel as if a Reviewed-by tag is appropriate.

~Andrew

> ---
>  tools/libxl/gentest.py |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
> index 6fab493..722b7f4 100644
> --- a/tools/libxl/gentest.py
> +++ b/tools/libxl/gentest.py
> @@ -42,6 +42,7 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
>      elif isinstance(ty, idl.KeyedUnion):
>          if parent is None:
>              raise Exception("KeyedUnion type must have a parent")
> +        s += gen_rand_init(ty.keyvar.type, parent + ty.keyvar.name, indent, parent)
>          s += "switch (%s) {\n" % (parent + ty.keyvar.name)
>          for f in ty.fields:
>              (nparent,fexpr) = ty.member(v, f, parent is None)

Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union

[Ian Campbell]

On Wed, 2013-12-04 at 17:54 +0000, Andrew Cooper wrote:
> On 04/12/13 17:48, Ian Campbell wrote:
> > This is Coverity CID 1135378 and 1135379.
> >
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> This sound plausible, although given my unfamiliarity with gentest.py, I
> dont feel as if a Reviewed-by tag is appropriate.

FWIW, if I insert a random.random() at the same place (to account for
the new call to random to be introduced, which knocks all the subsequent
values "down one") and then compare the result of that with the output
of this patch (with LIBXL_TESTIDL_SEED=42 in both cases) the diff is:

--- tools/libxl/testidl_BACKUP.c	2013-12-05 09:22:07.000000000 +0000
+++ tools/libxl/testidl.c	2013-12-05 09:22:49.000000000 +0000
@@ -439,6 +439,7 @@ static void libxl_domain_build_info_rand
     }
     libxl_defbool_rand_init(&p->claim_mode);
     p->event_channels = rand() % (sizeof(p->event_channels)*8);
+    p->type = LIBXL_DOMAIN_TYPE_INVALID;
     switch (p->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         p->u.hvm.firmware = rand_str();
@@ -739,6 +740,7 @@ static void libxl_event_rand_init(libxl_
     libxl_domid_rand_init(&p->domid);
     libxl_uuid_rand_init(&p->domuuid);
     p->for_user = rand() % (sizeof(p->for_user)*8);
+    p->type = LIBXL_EVENT_TYPE_DOMAIN_CREATE_CONSOLE_AVAILABLE;
     switch (p->type) {
     case LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN:
         p->u.domain_shutdown.shutdown_reason = rand() % (sizeof(p->u.domain_shutdown.shutdown_reason)*8);

Since this is a) not critical code by any stretch and b) not something I
expect anyone is going to review, I've just applied it.

Ian

Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union"):
> On Wed, 2013-12-04 at 17:54 +0000, Andrew Cooper wrote:
> > This sound plausible, although given my unfamiliarity with gentest.py, I
> > dont feel as if a Reviewed-by tag is appropriate.
> 
> FWIW, if I insert a random.random() at the same place (to account for
> the new call to random to be introduced, which knocks all the subsequent
> values "down one") and then compare the result of that with the output
> of this patch (with LIBXL_TESTIDL_SEED=42 in both cases) the diff is:

I did try to review this, but I wasn't sure how to do that random
alignment trick and then I saw you'd applied it.

> Since this is a) not critical code by any stretch and b) not something I
> expect anyone is going to review, I've just applied it.

Fair enough.

Thanks,
Ian.