On Tue, Sep 10, 2024 at 5:57 AM Eelco Chaudron <[email protected]> wrote:
>
>
>
> On 9 Sep 2024, at 6:55, Mike Pattrick wrote:
>
> > Currently the error conditions in vconn_dump_flows() don't handle
> > freeing memory in a consistent fashion. This can make it possible to
> > reference memory after it's freed.
> >
> > This patch attempts to handle errors consistently. Error conditions will
> > always cause memory to be freed and then that memory will never be
> > referenced.
> >
> > Fixes: d444a914fdbd ("ovn-trace: New --ovs option to also print OpenFlow
> > flows.")
> > Signed-off-by: Mike Pattrick <[email protected]>
>
> The changes look good to me.
>
> Acked-by: Eelco Chaudron <[email protected]>
>
> //Eelco
>
> Had to spend some time figuring out why ‘*replyp = NULL;’ had to be moved out
> of the !more part…
The control flow of these functions is pretty confusing. Aside from
being needed, I think there's also some stylistic benefit to always
having delete next to setting the pointer to NULL.
-M
>
> > ---
> > v3: Simplified logic per review.
> > ---
> > lib/vconn.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/vconn.c b/lib/vconn.c
> > index e9603432d..4b1c262ea 100644
> > --- a/lib/vconn.c
> > +++ b/lib/vconn.c
> > @@ -1017,6 +1017,8 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> > send_xid,
> > VLOG_WARN_RL(&rl, "received bad reply: %s",
> > ofp_to_string(reply->data, reply->size,
> > NULL, NULL, 1));
> > + ofpbuf_delete(reply);
> > + *replyp = NULL;
> > return EPROTO;
> > }
> > }
> > @@ -1031,9 +1033,9 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> > send_xid,
> > case EOF:
> > more = ofpmp_more(reply->header);
> > ofpbuf_delete(reply);
> > + *replyp = NULL;
> > reply = NULL;
> > if (!more) {
> > - *replyp = NULL;
> > return EOF;
> > }
> > break;
> > @@ -1041,6 +1043,8 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> > send_xid,
> > default:
> > VLOG_WARN_RL(&rl, "parse error in reply (%s)",
> > ofperr_to_string(retval));
> > + ofpbuf_delete(reply);
> > + *replyp = NULL;
> > return EPROTO;
> > }
> > }
> > --
> > 2.43.5
> >
> > _______________________________________________
> > 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
