Re: libxl gentypes.pl "saved_FOO" oddity

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Apr 04, 2016 at 03:57:02PM +0100, Ian Jackson wrote:
> Coverity is complaining (eg, CID 1358114) about this code in
> _libxl_types.c:
> 
>  *** CID 1358114:  Code maintainability issues  (UNUSED_VALUE)
>  /tools/libxl/_libxl_types.c: 11035 in libxl__device_usbdev_parse_json()
>  11029     x = libxl__json_map_get("hostaddr", x, JSON_INTEGER);
>  11030     if (x) {
>  11031         rc = libxl__uint8_parse_json(gc, x, &p->u.hostdev.hostaddr);
>  11032         if (rc)
>  11033             goto out;
>  11034     }
>  >>>     CID 1358114:  Code maintainability issues  (UNUSED_VALUE)
>  >>>     Assigning value from "saved_hostaddr" to "x" here, but that stored vale is overwritten before it can be used.
>  11035     x = saved_hostaddr;
> 
> This does seem rather odd.  I wasn't able to find any occurrences of
> `x' outside these save/restore regions.  So x is saved and restored
> for no particular reason.
> 
> The root cause seems to be the reuse of x by both inner and outer
> autogenerated code, where the generator may not know what is to be
> inserted.  Why not have a separate variable for each
> libxl__json_map_get ?
> 
> This code is generated by gentypes.py, near line 438.  It was written
> by Ian Campbell in 2010 and doesn't seem to have been much touched
> since.
> 

Actually I wrote that snippet back then. Maybe at the time I meant to
use `x' as a pointer of the node being processed for no particular
reason.

> Opinions welcome.  In particular, should I attempt a patch to make
> this code less odd-looking ?
> 

Sure. I don't object to this.

Wei.

> Thanks,
> Ian.
> 
> (CCing Wei who seems from git log like the only person who might have
> a view.)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel