On 29 April 2016 at 09:53, William Tu <u9012...@gmail.com> wrote: > 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
I think you are right. Thanks for pointing out those patches. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
