[libvirt] [PATCH] nwfilter: cleanup return codes in nwfilter subsystem

[Eric Blake]

On 11/23/2011 02:19 PM, Stefan Berger wrote:
> This patch cleans up return codes in the nwfilter subsystem.
> 
> Some functions in nwfilter_conf.c (validators and formatters) are
> keeping their bool return for now and I am converting their return
> code to true/false.
> 
> All other functions now return -1 on failure and 0 on success.
> 
> [I searched for all occurences of ' 1;' and checked all 'if ' and
> adapted where needed. After that I did a grep for 'NWFilter' in the source
> tree.]
> 
> ---

I compile tested this, and assume you also ran it through libvirt-TCK.
My compile run complained:

qemu/qemu_command.c: In function 'qemuNetworkIfaceConnect':
qemu/qemu_command.c:278:65: error: suggest braces around empty body in
an 'if' statement [-Werror=empty-body]

[more on this below]

> -static bool
> +static int
>  intMapGetByInt(const struct int_map *intmap, int32_t attr, const char **res)
>  {
>      int i = 0;
> -    bool found = 0;
> +    int found = false;

This should still be 'bool found', given how you assign bool values to it.

> -static bool
> +static int
>  intMapGetByString(const struct int_map *intmap, const char *str, int casecmp,
>                    int32_t *result)

Conversion of intMapGetBy{Int,String} is correct, including all callers.
 (I'm not sure if this would have been easier broken up into multiple
patches, a couple functions per patch, but don't bother splitting it now).

> @@ -367,14 +369,14 @@ virNWFilterRuleDefAddVar(virNWFilterRule
>  
>      if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1) < 0) {
>          virReportOOMError();
> -        return 1;
> +        return -1;

Conversion of virNWFilterRuleDefAddVar is correct, including all callers.

> @@ -805,7 +808,7 @@ parseStringItems(const struct int_map *i
>              }
>          }
>          if (!found) {
> -            rc = 1;
> +            rc = -1;

You still have two callers to parseStringItems that merely checked for
non-zero, rather than < 0 (in tcpFlagsValidator), but no logic
regression introduced.

> @@ -1663,13 +1666,11 @@ static const virAttributes virAttr[] = {
>  };
>  
>  
> -static bool
> +static int
>  virNWMACAddressParser(const char *input,
>                        nwMACAddressPtr output)
>  {
> -    if (virParseMacAddr(input, &output->addr[0]) == 0)
> -        return 1;
> -    return 0;
> +    return virParseMacAddr(input, &output->addr[0]);

Conversion of virNWMACAddressParser is correct.

> @@ -2649,7 +2650,7 @@ _virNWFilterDefLoopDetect(virConnectPtr 
>          if (entry->include) {
>  
>              if (STREQ(filtername, entry->include->filterref)) {
> -                rc = 1;
> +                rc = -1;

conversion of [_]virNWFilterDefLoopDetect and callers looks correct.

> @@ -2736,7 +2737,7 @@ virNWFilterTriggerVMFilterRebuild(virCon
>      };
>  
>      if (!cb.skipInterfaces)
> -        return 1;
> +        return -1;
>  
>      for (i = 0; i < nCallbackDriver; i++) {
>          callbackDrvArray[i]->vmFilterRebuild(conn,

This one's tricky - you have err = cb.err, but I didn't check whether
all callers to virNWFilterConfLayerInit() pass a function that sets
cb.err to -1, or if you still need more cleanup.


> @@ -2778,7 +2779,7 @@ virNWFilterTestUnassignDef(virConnectPtr
>      nwfilter->wantRemoved = 1;
>      /* trigger the update on VMs referencing the filter */
>      if (virNWFilterTriggerVMFilterRebuild(conn))
> -        rc = 1;
> +        rc = -1;

Conversion of virNWFilterTestUnassignDef and callers looks correct.

> @@ -3297,8 +3298,8 @@ int virNWFilterConfLayerInit(virHashIter
>  
>      initialized = true;
>  
> -    if (virMutexInitRecursive(&updateMutex))
> -        return 1;
> +    if (virMutexInitRecursive(&updateMutex) < 0)
> +        return -1;

Conversion of virNWFilterConfLayerInit() and callers looks correct.

>  
>      return 0;
>  }
> Index: libvirt-acl/src/nwfilter/nwfilter_driver.c

This is as far as I got today.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111123/7806124c/attachment-0001.sig>