On Tue, Jun 28, 2011 at 09:03:52PM +0200, Jim Meyering wrote:
Hi Rich,
While I was looking at hivex today I ran coverity on it.
It spotted one problem but missed a similar one nearby.
The following are from Hivex.xs: (generated by generator.ml)
void
node_set_values (h, node, values)
hive_h *h;
int node;
pl_set_values values = unpack_pl_set_values (ST(2));
PREINIT:
int r;
PPCODE:
r = hivex_node_set_values (h, node, values.nr_values, values.values, 0);
free (values.values);
if (r == -1)
croak ("%s: %s", "node_set_values", strerror (errno));
void
node_set_value (h, node, val)
hive_h *h;
int node;
hive_set_value *val = unpack_set_value (ST(2));
PREINIT:
int r;
PPCODE:
r = hivex_node_set_value (h, node, val, 0);
free (val);
if (r == -1)
croak ("%s: %s", "node_set_value", strerror (errno));
--------------------------------------------
Here's the generated C.
Note how each function uses XSRETURN_UNDEF
without freeing the "values" or "val" data they
have just allocated:
XS(XS_Win__Hivex_node_set_values); /* prototype to pass -Wmissing-prototypes */
XS(XS_Win__Hivex_node_set_values)
{
#ifdef dVAR
dVAR; dXSARGS;
#else
dXSARGS;
#endif
if (items != 3)
croak_xs_usage(cv, "h, node, values");
PERL_UNUSED_VAR(ax); /* -Wall */
SP -= items;
{
hive_h * h;
int node = (int)SvIV(ST(1));
pl_set_values values = unpack_pl_set_values (ST(2));
#line 477 "Hivex.xs"
int r;
#line 993 "Hivex.c"
if (sv_isobject (ST(0)) && SvTYPE (SvRV (ST(0))) == SVt_PVMG)
h = (hive_h *) SvIV ((SV *) SvRV (ST(0)));
else {
warn ("Win::Hivex::node_set_values(): h is not a blessed SV
reference");
XSRETURN_UNDEF;
};
#line 479 "Hivex.xs"
r = hivex_node_set_values (h, node, values.nr_values, values.values, 0);
free (values.values);
if (r == -1)
croak ("%s: %s", "node_set_values", strerror (errno));
#line 1006 "Hivex.c"
PUTBACK;
return;
}
}
XS(XS_Win__Hivex_node_set_value); /* prototype to pass -Wmissing-prototypes */
XS(XS_Win__Hivex_node_set_value)
{
#ifdef dVAR
dVAR; dXSARGS;
#else
dXSARGS;
#endif
if (items != 3)
croak_xs_usage(cv, "h, node, val");
PERL_UNUSED_VAR(ax); /* -Wall */
SP -= items;
{
hive_h * h;
int node = (int)SvIV(ST(1));
hive_set_value * val = unpack_set_value (ST(2));
#line 490 "Hivex.xs"
int r;
#line 1031 "Hivex.c"
if (sv_isobject (ST(0)) && SvTYPE (SvRV (ST(0))) == SVt_PVMG)
h = (hive_h *) SvIV ((SV *) SvRV (ST(0)));
else {
warn ("Win::Hivex::node_set_value(): h is not a blessed SV
reference");
XSRETURN_UNDEF;
};
#line 492 "Hivex.xs"
r = hivex_node_set_value (h, node, val, 0);
free (val);
if (r == -1)
croak ("%s: %s", "node_set_value", strerror (errno));
#line 1044 "Hivex.c"
PUTBACK;
return;
}
}
--------------------------------------------
One way to fix it is to change generator.ml to induce this
change in Hivex.xs:
--- Hivex.xs.~1~ 2011-06-28 21:01:28.374623171 +0200
+++ Hivex.xs 2011-06-28 21:01:43.351623367 +0200
@@ -472,10 +472,10 @@ void
node_set_values (h, node, values)
hive_h *h;
int node;
- pl_set_values values = unpack_pl_set_values (ST(2));
PREINIT:
int r;
PPCODE:
+ pl_set_values values = unpack_pl_set_values (ST(2));
r = hivex_node_set_values (h, node, values.nr_values, values.values, 0);
free (values.values);
if (r == -1)
@@ -485,10 +485,10 @@ void
node_set_value (h, node, val)
hive_h *h;
int node;
- hive_set_value *val = unpack_set_value (ST(2));
PREINIT:
int r;
PPCODE:
+ hive_set_value *val = unpack_set_value (ST(2));
r = hivex_node_set_value (h, node, val, 0);
free (val);
if (r == -1)
Tricky change to the generator. I'll have to think about this a bit
more tomorrow ...
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top