[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] util: use VIR_(APPEND|DELETE)_ELEMENT for pci/usb device lists



On 05.07.2013 20:46, Laine Stump wrote:
> Eliminate memmove() by using VIR_*_ELEMENT API instead.
> 
> In both pci and usb cases, the count that held the size of the list
> was unsigned int so it had to be changed to size_t.
> ---
> This is an alternate fix to the same problem fixed by
> 
>   https://www.redhat.com/archives/libvir-list/2013-July/msg00324.html
> 
> (an off by one bug in the conditional in virUSBDeviceListSteal()).
> 
> This version fixes it by converting from open coded memmoves to using
> VIR_(INSERT|DELETE)_ELEMENT.
> 
>  src/util/virpci.c | 18 +++---------------
>  src/util/virusb.c | 26 +++++++-------------------
>  2 files changed, 10 insertions(+), 34 deletions(-)
> 
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index e42f6fa..b715122 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -80,7 +80,7 @@ struct _virPCIDevice {
>  struct _virPCIDeviceList {
>      virObjectLockable parent;
>  
> -    unsigned int count;
> +    size_t count;
>      virPCIDevicePtr *devs;
>  };
>  
> @@ -1670,11 +1670,9 @@ virPCIDeviceListAdd(virPCIDeviceListPtr list,
>          return -1;
>      }
>  
> -    if (VIR_REALLOC_N(list->devs, list->count+1) < 0)
> +    if (VIR_APPEND_ELEMENT(list->devs, list->count, dev, true) < 0)
>          return -1;
>  
> -    list->devs[list->count++] = dev;
> -
>      return 0;
>  }
>  
> @@ -1723,17 +1721,7 @@ virPCIDeviceListStealIndex(virPCIDeviceListPtr list,
>          return NULL;
>  
>      ret = list->devs[idx];
> -
> -    if (idx != --list->count) {
> -        memmove(&list->devs[idx],
> -                &list->devs[idx + 1],
> -                sizeof(*list->devs) * (list->count - idx));
> -    }
> -
> -    if (VIR_REALLOC_N(list->devs, list->count) < 0) {
> -        ; /* not fatal */
> -    }
> -
> +    VIR_DELETE_ELEMENT(list->devs, idx, list->count);
>      return ret;
>  }
>  
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index 3bcb07c..103c301 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -60,7 +60,7 @@ struct _virUSBDevice {
>  
>  struct _virUSBDeviceList {
>      virObjectLockable parent;
> -    unsigned int count;
> +    size_t count;
>      virUSBDevicePtr *devs;
>  };
>  
> @@ -451,11 +451,9 @@ virUSBDeviceListAdd(virUSBDeviceListPtr list,
>          return -1;
>      }
>  
> -    if (VIR_REALLOC_N(list->devs, list->count+1) < 0)
> +    if (VIR_APPEND_ELEMENT(list->devs, list->count, dev, true) < 0)
>          return -1;
>  
> -    list->devs[list->count++] = dev;
> -
>      return 0;
>  }
>  
> @@ -484,22 +482,12 @@ virUSBDeviceListSteal(virUSBDeviceListPtr list,
>      int i;
>  
>      for (i = 0; i < list->count; i++) {
> -        if (list->devs[i]->bus != dev->bus ||
> -            list->devs[i]->dev != dev->dev)
> -            continue;
> -
> -        ret = list->devs[i];
> -
> -        if (i != list->count--)
> -            memmove(&list->devs[i],
> -                    &list->devs[i+1],
> -                    sizeof(*list->devs) * (list->count - i));
> -
> -        if (VIR_REALLOC_N(list->devs, list->count) < 0) {
> -            ; /* not fatal */
> +        if (list->devs[i]->bus == dev->bus &&
> +            list->devs[i]->dev == dev->dev) {
> +            ret = list->devs[i];
> +            VIR_DELETE_ELEMENT(list->devs, i, list->count);
> +            break;
>          }
> -
> -        break;
>      }
>      return ret;
>  }
> 


ACK

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]