[ovs-dev] [race-fix 2/6] classifier: New function cls_rule_move().

[Ethan Jackson]

Much clearer, thanks.

Ethan

On Tue, Aug 27, 2013 at 12:25 PM, Ben Pfaff <blp at nicira.com> 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.