[libvirt] [PATCH v2 01/14] test: Adjust cleanup/error paths for nodedev test APIs

[Peter Krempa]

On Thu, May 25, 2017 at 15:56:58 -0400, John Ferlan wrote:
>  - Rather than "goto cleanup;" on failure to virNodeDeviceObjFindByName
>    an @obj, just return directly.  This then allows the cleanup: label code
>    to not have to check "if (obj)" before calling virNodeDeviceObjUnlock.
>    This also simplifies some exit logic...
> 
>  - In testNodeDeviceObjFindByName use an error: label to handle the failure
>    and don't do the ncaps++ within the VIR_STRDUP() source target index.
>    Only increment ncaps after success. Easier on eyes at error label too.
> 
>  - In testNodeDeviceDestroy use "cleanup" rather than "out" for the goto
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/test/test_driver.c | 75 +++++++++++++++++++-------------------------------
>  1 file changed, 29 insertions(+), 46 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 2db3f7d..3389edd 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4512,7 +4512,6 @@ testDestroyVport(testDriverPtr privconn,
>                   const char *wwnn ATTRIBUTE_UNUSED,
>                   const char *wwpn ATTRIBUTE_UNUSED)
>  {
> -    int ret = -1;
>      virNodeDeviceObjPtr obj = NULL;
>      virObjectEventPtr event = NULL;
>  
> @@ -4526,7 +4525,7 @@ testDestroyVport(testDriverPtr privconn,
>      if (!(obj = virNodeDeviceObjFindByName(&privconn->devs, "scsi_host12"))) {
>          virReportError(VIR_ERR_NO_NODE_DEVICE, "%s",
>                         _("no node device with matching name 'scsi_host12'"));
> -        goto cleanup;
> +        return -1;
>      }
>  
>      event = virNodeDeviceEventLifecycleNew("scsi_host12",
> @@ -4535,13 +4534,8 @@ testDestroyVport(testDriverPtr privconn,
>  
>      virNodeDeviceObjRemove(&privconn->devs, &obj);

A static analyzer may argue that 'obj' may be non-null in some cases at
this point ...

>  
> -    ret = 0;
> -
> - cleanup:
> -    if (obj)
> -        virNodeDeviceObjUnlock(obj);

So this would not unlock it.

>      testObjectEventQueue(privconn, event);
> -    return ret;
> +    return 0;
>  }

ACK to the rest
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170526/ee5c65bc/attachment-0001.sig>