[ovs-dev] [RFC PATCH 2/8] ofpacts_check: Remove unnecessary flow copying.

Ben Pfaff blp at nicira.com
Mon Jun 24 17:13:37 UTC 2013


On Mon, Jun 24, 2013 at 03:26:52PM +0000, Rajahalme, Jarno (NSN - FI/Espoo) wrote:
> 
> On Jun 20, 2013, at 23:36 , ext Ben Pfaff wrote:
> 
> > On Thu, Jun 20, 2013 at 05:26:17PM +0300, Jarno Rajahalme wrote:
> >> 
> >> Signed-off-by: Jarno Rajahalme <jarno.rajahalme at nsn.com>
> > 
> > I have mixed feelings here.  On one hand, it is nice to avoid a big
> > copy in some cases (rare cases really).  But on the other hand I am
> > not fond of interfaces that temporarily modify data, even if they
> > change it back later, and the pitfalls of such an approach are more of
> > a problem as threading comes into the picture (and we are currently in
> > the midst of threading OVS).
> > 
> > What do you think?
> 
> 
> I'm in favor of avoiding unnecessary copying of big data items. I
> guess my uneasiness limit would be crossed by taking in a const
> pointer and then casting and modifying the data anyway. So as long
> as the API is honest, and there is no need for a function to be
> re-entrant on the same data, I'm OK with functions modifying the
> data they are given.  The callers use this function with private
> data in stack, so I think there is very low risk of this being a
> threading problem.

You make a good point that at least the function signature is honest
about it.

I applied this.  I adjusted it slightly (moved a comment, changed a
"goto" to a "break").

Thanks,

Ben.



More information about the dev mailing list