libxl gentypes.pl "saved_FOO" oddity

libxl gentypes.pl "saved_FOO" oddity

[Ian Jackson]

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.

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

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

Re: libxl gentypes.pl "saved_FOO" oddity

[Wei Liu]

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