On May 12, 2014, at 8:48 AM, Ben Pfaff <b...@nicira.com> wrote: > On Mon, May 12, 2014 at 08:43:10AM -0700, Jarno Rajahalme wrote: >> >> On May 12, 2014, at 8:36 AM, Ben Pfaff <b...@nicira.com> wrote: >> >>> On Sun, May 11, 2014 at 11:55:01PM -0700, Jarno Rajahalme wrote: >>>> Array splicing was broken when multiple elements were being moved, >>>> resulting in the priority order being mixed. This came up when the >>>> highest priority rule from a subtable was removed and the subtable >>>> needed to be moved down the priority list by more than one position. >>>> >>>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> >>> >>> Can you explain what this function does? I feel like it should be >>> obvious (I mean, clearly it's reordering the array), but in fact I'm >>> having trouble puzzling it out. >>> >> >> It moves elements starting from ?start? and ending before ?end? to >> ?to?. This is the same as swapping the places of items [to,start) and >> [start,end), when to < start. I implemented shifting to one direction >> only, so in the buggy case the arguments are shifted like this: to = >> start, start = end and end = (old) to. >> >> The loop moves elements one by one, moving the items [to, start) >> forward by one, and moving the (old) ?start' element to ?to?. I forgot >> to advance the ?to? at each round... > > Why isn't this just a single big memmove()?
I did not allocate memory for more than one element (temp) and in the general case a loop is needed with this limitation. The user of this function used to call the list splice function and I wanted to keep the call site similar when changing to an array. Now, we ever only move one element, so it could be a single memmove, in the upper part of the function, moving the block towards the beginning. That could be a separate patch, but I did not want to obfuscate the bug fix at this time. Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
