Looks good to me.
I had to stop and think a little bit about the ofpact_finish()
> function's API. It gives freedom to its caller to specify whatever it
> wants as second 'ofpact' argument. However, at the end of the day
> ofpact_finish() asserts if second argument value does not match to the
> first argument's "->header" value. I think that in theory this
> function really needs only the first argument to do its job, because
> second argument is really implied from the first argument.
>
> BTW, there are other bugs in similar nature in this same file, for
> example, the 'bundle' variable is still referenced after invoking
> ofpact_finish_BUNDLE().
>
This one is OK. because when
ofpact_finish_BUNDLE(ofpacts, &bundle);
the address 'bundle' points to will be updated if reallocation happens. So
we need the second argument as a way to update the stale pointer.
This is mentioned here:
https://patchwork.ozlabs.org/patch/593747/
http://openvswitch.org/pipermail/dev/2016-April/069508.html
Regards,
William
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
