Re: [PATCH 2/2] vvfat: Fix array_remove_slice()

[Kevin Wolf <kwolf@redhat.com>]

Am 23.06.2020 um 20:30 hat Eric Blake geschrieben:
> On 6/23/20 12:55 PM, Kevin Wolf wrote:
> > array_remove_slice() calls array_roll() with array->next - 1 as the
> > destination index. This is only correct for count == 1, otherwise we're
> > writing past the end of the array. array->next - count would be correct.
> > 
> > However, this is the only place ever calling array_roll(), so this
> > rather complicated operation isn't even necessary.
> > 
> > Fix the problem and simplify the code by replacing it with a single
> > memmove() call. array_roll() can now be removed.
> > 
> > Reported-by: Nathan Huckleberry <nhuck15@gmail.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   block/vvfat.c | 42 +++++-------------------------------------
> >   1 file changed, 5 insertions(+), 37 deletions(-)
> > 
> > diff --git a/block/vvfat.c b/block/vvfat.c
> > index 2fab371258..d6e464c595 100644
> > --- a/block/vvfat.c
> > +++ b/block/vvfat.c
> > @@ -140,48 +140,16 @@ static inline void* array_insert(array_t* array,unsigned int index,unsigned int
> >       return array->pointer+index*array->item_size;
> >   }
> > -/* this performs a "roll", so that the element which was at index_from becomes
> > - * index_to, but the order of all other elements is preserved. */
> > -static inline int array_roll(array_t* array,int index_to,int index_from,int count)
> 
> If I understand the intent from just the comment, the old code would take a
> directory listing of six files:
> 
> ABCDEF
> 
> and on the request to delete file C, would produce:
> 
> ABFDE
> 
> by moving just F, instead of all of DEF.

I think what the old code did was actually moving C, not F, so you get:

ABDEFC

And then you reduce the array size by one so that C isn't visible any
more. My code preserves this behaviour, except that the invisible final
element is a copy of F instead C now.

Kevin