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

Re: [libvirt] [PATCH] util: Use new array management macros



On 01/07/2014 08:03 AM, Osier Yang wrote:
> Like commit 94a26c7e from Eric Blake, the old fuzzy code should
> be replaced by the new array management macros now.
> 
> And the type of scsi->count should be changed into "size_t", and
> thus virSCSIDeviceListCount should return size_t instead, similar
> for vir{PCI,USB}DeviceListCount.
> ---
>  src/util/virpci.c  |  2 +-
>  src/util/virpci.h  |  2 +-
>  src/util/virscsi.c | 35 ++++++++++-------------------------
>  src/util/virscsi.h |  2 +-
>  src/util/virusb.c  |  2 +-
>  src/util/virusb.h  |  2 +-
>  6 files changed, 15 insertions(+), 30 deletions(-)

Not quite.

> @@ -387,24 +382,14 @@ virSCSIDeviceListSteal(virSCSIDeviceListPtr list,
>      size_t i;
>  
>      for (i = 0; i < list->count; i++) {
> -        if (list->devs[i]->adapter != dev->adapter ||
> -            list->devs[i]->bus != dev->bus ||
> -            list->devs[i]->target != dev->target ||
> -            list->devs[i]->unit != dev->unit)

In the old code, if 3 of the 4 items match, but the fourth does not,
then we skip that item.

> -            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]->adapter == dev->adapter ||
> +            list->devs[i]->bus == dev->bus ||
> +            list->devs[i]->target == dev->target ||
> +            list->devs[i]->unit == dev->unit) {

In the new code as written, only 1 of the 4 items has to match.  You
applied deMorgan's theorem incorrectly; this must be:

if (list->devs[i]->adapter == dev->adapter && ...

ACK if you replace the three || with &&.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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