Much clearer, thanks. Ethan
On Tue, Aug 27, 2013 at 12:25 PM, Ben Pfaff <[email protected]> wrote: > On Mon, Aug 26, 2013 at 06:07:08PM -0700, Ethan Jackson wrote: >> > +/* Replaces 'dst' by 'src', destroying 'src'. The caller must eventually >> > free >> > + * 'dst' with miniflow_destroy(). */ >> > +void >> > +miniflow_move(struct miniflow *dst, struct miniflow *src) >> > +{ >> > + int n = miniflow_n_values(src); >> > + if (n <= MINI_N_INLINE) { >> > + dst->values = dst->inline_values; >> > + memcpy(dst->values, src->values, n * sizeof *dst->values); >> >> Does this leak memory in the case where miniflow_n_values(dst) > >> MINI_N_INLINE? Later when we destroy dst, we don't know that we >> should be freeing dst->values because dst->values == >> dst->inline_values. Or, are we assuming that dst hasn't already been >> initialized? In that case I think the function comment on this and >> cls_rule_move() could be expanded. > > The latter. > > I changed the comment on this function to: > /* Initializes 'dst' with the data in 'src', destroying 'src'. > * The caller must eventually free 'dst' with miniflow_destroy(). */ > I also made similar changes to the other new functions. Is that > clearer? I can add another sentence if you do not think so. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
