On 07/05/2013 02:23 AM, Gonglei (Arei) wrote: > In the for loop, the if condition is always true, and will execute memmove. > But it will cause the list->devs[i+1] overflow while i equals list->count-1. > > Signed-off-by: Gonglei <arei.gonglei@xxxxxxxxxx> > --- > src/util/virusb.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/util/virusb.c b/src/util/virusb.c > index d34e44f..30d0b12 100644 > --- a/src/util/virusb.c > +++ b/src/util/virusb.c > @@ -497,7 +497,7 @@ virUSBDeviceListSteal(virUSBDeviceListPtr list, > > ret = list->devs[i]; > > - if (i != list->count--) > + if (i != --list->count) > memmove(&list->devs[i], > &list->devs[i+1], > sizeof(*list->devs) * (list->count - i)); This function is a good candidate for switching to VIR_DELETE_ELEMENT() instead. This will eliminate the bug that you found while making the code much shorter. I have a patch for that sitting around, I'll rebase it and post it. (I actually have 15 patches that replace memmove+VIR_REALLOC() with VIR_DELETE_ELEMENT and VIR_INSERT_ELEMENT. I had written them as examples of VIR_*_ELEMENT and posted them, but was too nervous of potential regression to push them (or even nag about reviews). -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list