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
Ian Campbell
2013-Dec-04 17:48 UTC
[PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union
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
Andrew Cooper
2013-Dec-04 17:54 UTC
Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union
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)
Ian Campbell
2013-Dec-05 11:39 UTC
Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union
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
Ian Jackson
2013-Dec-10 14:12 UTC
Re: [PATCH] tools: libxl: testidl: initialise the KeyedUnion keyvar before the union
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.