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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
