Re: [PATCH] cleanup: clamp max info with MIN().

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Thu, May 15, 2014 at 08:03:09PM +0900, Dongsheng Yang wrote:
> Rather than using a open coded implementation,
> this patch use MIN macro to clamp infomation
> to allowed maxmum.

Sorry to say that, but I don't find it as readable as before.  At
first, the idea with CLAMP was a nice improvement in case of both ends
of the range being utilized, but I don't see any added value in this
particular patch.

I was waiting if somebody expresses their opinion on this patch, but
no response yet.  If there is particular need for this patch (or it
helps with something I don't see), let me know, but if not then I'd
leave it as is.  Mainly because some future patches will most likely
not use MIN() and we would have another inconsistency in the code.

Martin

> Signed-off-by: Dongsheng Yang <yangds.fnst@xxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 18 ++++++------------
> src/test/test_driver.c | 10 +++-------
> 2 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 52ca47c..b3782dd 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4542,12 +4542,9 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom,
>         goto cleanup;
>
>     maxcpu = maplen * 8;
> -    if (maxcpu > hostcpus)
> -        maxcpu = hostcpus;
>
> -    /* Clamp to actual number of vcpus */
> -    if (ncpumaps > targetDef->vcpus)
> -        ncpumaps = targetDef->vcpus;
> +    maxcpu = MIN(maxcpu, hostcpus);
> +    ncpumaps = MIN(ncpumaps, targetDef->vcpus);
>
>     if (ncpumaps < 1) {
>         goto cleanup;
> @@ -4786,8 +4783,8 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom,
>         goto cleanup;
>
>     maxcpu = maplen * 8;
> -    if (maxcpu > hostcpus)
> -        maxcpu = hostcpus;
> +
> +    maxcpu = MIN(maxcpu, hostcpus);
>
>     /* initialize cpumaps */
>     memset(cpumaps, 0xff, maplen);
> @@ -4852,12 +4849,9 @@ qemuDomainGetVcpus(virDomainPtr dom,
>         goto cleanup;
>
>     maxcpu = maplen * 8;
> -    if (maxcpu > hostcpus)
> -        maxcpu = hostcpus;
>
> -    /* Clamp to actual number of vcpus */
> -    if (maxinfo > priv->nvcpupids)
> -        maxinfo = priv->nvcpupids;
> +    maxcpu = MIN(maxcpu, hostcpus);
> +    maxinfo = MIN(maxinfo, priv->nvcpupids);
>
>     if (maxinfo >= 1) {
>         if (info != NULL) {
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 37756e7..4e591f2 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2773,12 +2773,9 @@ static int testDomainGetVcpus(virDomainPtr domain,
>
>     hostcpus = VIR_NODEINFO_MAXCPUS(privconn->nodeInfo);
>     maxcpu = maplen * 8;
> -    if (maxcpu > hostcpus)
> -        maxcpu = hostcpus;
>
> -    /* Clamp to actual number of vcpus */
> -    if (maxinfo > privdom->def->vcpus)
> -        maxinfo = privdom->def->vcpus;
> +    maxcpu = MIN(maxcpu, hostcpus);
> +    maxinfo = MIN(maxinfo, privdom->def->vcpus);
>
>     /* Populate virVcpuInfo structures */
>     if (info != NULL) {
> @@ -2858,8 +2855,7 @@ static int testDomainPinVcpu(virDomainPtr domain,
>     privmaplen = VIR_CPU_MAPLEN(hostcpus);
>
>     maxcpu = maplen * 8;
> -    if (maxcpu > hostcpus)
> -        maxcpu = hostcpus;
> +    maxcpu = MIN(maxcpu, hostcpus);
>
>     privcpumap = VIR_GET_CPUMAP(privdomdata->cpumaps, privmaplen, vcpu);
>     memset(privcpumap, 0, privmaplen);
> --
> 1.8.2.1
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list