[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] vvfat: Fix array_remove_slice()
From: |
Kevin Wolf |
Subject: |
Re: [PATCH 2/2] vvfat: Fix array_remove_slice() |
Date: |
Wed, 24 Jun 2020 14:42:43 +0200 |
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