This patch has an issue. I'm working on a v2. --Justin
> On Jun 27, 2017, at 5:35 PM, Justin Pettit <[email protected]> wrote: > > The function parse_ofp_meter_mod_str() allocates a buffer called > 'bands', which parse_ofp_meter_mod_str__() then steals for the member > 'mm->meter.bands'. Calling functions didn't free that stolen value and > the comments for those function didn't indicate that was necessary. > > Found by valgrind. > > Signed-off-by: Justin Pettit <[email protected]> > --- > lib/ofp-parse.c | 11 +++++++---- > utilities/ovs-ofctl.c | 2 ++ > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c > index 528b75b4f4e1..725fc41e063c 100644 > --- a/lib/ofp-parse.c > +++ b/lib/ofp-parse.c > @@ -754,6 +754,8 @@ parse_ofp_packet_out_str(struct ofputil_packet_out *po, > const char *str_, > return error; > } > > +/* Parse a string representation of a meter modification message to '*mm'. > + * If successful, 'mm->meter.bands' must be free()d by the caller. */ > static char * OVS_WARN_UNUSED_RESULT > parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string, > struct ofpbuf *bands, int command, > @@ -795,6 +797,9 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, > char *string, > mm->command = command; > mm->meter.meter_id = 0; > mm->meter.flags = 0; > + mm->meter.n_bands = 0; > + mm->meter.bands = NULL; > + > if (fields & F_BANDS) { > band_str = strstr(string, "band"); > if (!band_str) { > @@ -945,9 +950,6 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, > char *string, > } > } > } > - } else { > - mm->meter.n_bands = 0; > - mm->meter.bands = NULL; > } > > return NULL; > @@ -957,7 +959,8 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, > char *string, > * page) into 'mm' for sending the specified meter_mod 'command' to a switch. > * > * Returns NULL if successful, otherwise a malloc()'d string describing the > - * error. The caller is responsible for freeing the returned string. */ > + * error. The caller is responsible for freeing the returned string. > + * If successful, 'mm->meter.bands' must be free()d by the caller. */ > char * OVS_WARN_UNUSED_RESULT > parse_ofp_meter_mod_str(struct ofputil_meter_mod *mm, const char *str_, > int command, enum ofputil_protocol *usable_protocols) > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > index ad862efe3e39..200ae9cb0078 100644 > --- a/utilities/ovs-ofctl.c > +++ b/utilities/ovs-ofctl.c > @@ -3704,6 +3704,7 @@ ofctl_meter_mod__(const char *bridge, const char *str, > int command) > protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols); > version = ofputil_protocol_to_ofp_version(protocol); > transact_noreply(vconn, ofputil_encode_meter_mod(version, &mm)); > + free(mm.meter.bands); > vconn_close(vconn); > } > > @@ -3732,6 +3733,7 @@ ofctl_meter_request__(const char *bridge, const char > *str, > version = ofputil_protocol_to_ofp_version(protocol); > dump_transaction(vconn, ofputil_encode_meter_request(version, type, > mm.meter.meter_id)); > + free(mm.meter.bands); > vconn_close(vconn); > } > > -- > 2.7.4 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
