[ovs-dev] [PATCH v2] miniflow: Remove unused values_inline branch from miniflow_move()

[Ben Pfaff]

On Tue, Aug 26, 2014 at 12:02:48PM -0700, Jarno Rajahalme wrote:
> 
> On Aug 26, 2014, at 3:01 AM, Thomas Graf <tgraf at noironetworks.com> wrote:
> 
> > The branch is unused as size < sizeof dst->inline_values must
> > always be true for inlined values. Hitting the branch would lead
> > to corruption as inline_values is accessed out of bounds.
> > 
> 
> Miniflows can also be dynamically allocated to have more inline space than the default amount. classifier.c makes use of that, but it never calls miniflow_move (even indirectly) so you are not getting an assert fail. The motivation for right-sized miniflows in the classifier is to reduce levels of indirection while not wasting memory.
> 
> I should (have) extend(ed) the comment above struct miniflow definition in lib/flow.h to describe the above.
> 
> size is computed from the actual source miniflow instance, not from the generic miniflow struct. If size really is larger than the default inline space, then obviously no out of bounds access would occur.
> 
> I?m OK changing this function to not work for dynamically allocated miniflows, but that should be reflected in the comment above the function, which currently reads:
> 
> /* Initializes 'dst' with the data in 'src', destroying 'src'.
>  * The caller must eventually free 'dst' with miniflow_destroy().
>  * 'dst' must be regularly sized miniflow, but 'src' can have
>  * larger than default inline values. */
> 
> However, it might be better to revert this patch to keep the larger-than-default miniflows not stand out too much.

Let's revert it and extend the comment above struct miniflow.