[libvirt] [PATCH 4/5] conf: eliminate redundant use of VIR_ALLOC

Osier Yang jyang at redhat.com
Tue Dec 18 03:21:26 UTC 2012


On 2012年12月17日 23:17, Martin Kletzander wrote:
> We can use VIR_REALLOC_N with NULL pointer, which behaves the same way
> as VIR_ALLOC_N in that case, so no need for a condition that's
> checking if some data are allocated already.
>
> ---
>
> I tried to find other parts of the code similar to this, so I can do a
> full cleanup for the whole repository, so I used this (excuse the long
> line, but that's how I was writing it):
>
> git grep -nHC 5 -e VIR_REALLOC_N -e VIR_ALLOC_N | while read line; do if [[ "$line" == "--" ]]; then if [[ ${#tmpbuf} -gt 10&&  "$REALLOC_N" == "true"&&  "$ALLOC_N" == "true" ]]; then echo $line; while [[ ${#tmpbuf[*]} -gt 0 ]]; do echo "${tmpbuf[0]}"; tmpbuf=( "${tmpbuf[@]:1:${#tmpbuf[*]}}" ); done; fi; unset tmpbuf REALLOC_N ALLOC_N; else if [[ "$ALLOC_N" != "true"&&  "${line/VIR_ALLOC_N//}" != "${line}" ]]; then ALLOC_N="true"; fi; if [[ "$REALLOC_N" != "true"&&  "${line/VIR_REALLOC_N//}" != "${line}" ]]; then REALLOC_N="true"; fi; tmpbuf[${#tmpbuf[*]}]="$line"; fi; done | less
>
> And reviewed the output just to find out this was the only occurrence of
> the inconsistency.
> ---
>   src/conf/domain_conf.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 329ada3..21d67a3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9047,16 +9047,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>        * the policy specified explicitly as def->cpuset.
>        */
>       if (def->cpumask) {
> -        if (!def->cputune.vcpupin) {
> -            if (VIR_ALLOC_N(def->cputune.vcpupin, def->vcpus)<  0) {
> -                virReportOOMError();
> -                goto error;
> -            }
> -        } else {
> -            if (VIR_REALLOC_N(def->cputune.vcpupin, def->vcpus)<  0) {
> -                virReportOOMError();
> -                goto error;
> -            }
> +        if (VIR_REALLOC_N(def->cputune.vcpupin, def->vcpus)<  0) {
> +            virReportOOMError();
> +            goto error;
>           }

I think sometimes one will still want the VIR_ALLOC_N, for it fills
the allocated memory with zeros, but not uninitialized values. it's
not common though. Or may be we should use VIR_EXPAND_N in that case.

But here we don't have to worry about the uninitialized values indeed.
So ACK.

Osier




More information about the libvir-list mailing list