[libvirt] Subject: [PATCH] libvirt-domain.c:virDomainMigrateCheckNotLocal function

xiajidong@cmss.chinamobile.com posted 1 patch 5 years, 7 months ago
Failed in applying to current master (apply log)
src/libvirt-domain.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
[libvirt] Subject: [PATCH] libvirt-domain.c:virDomainMigrateCheckNotLocal function
Posted by xiajidong@cmss.chinamobile.com 5 years, 7 months ago
>From c013b053d7514ee66b841bc99900b06d1e9d4dfd Mon Sep 17 00:00:00 2001
From: xiajidong <xiajidong@cmss.chinamobile.com>
Date: Tue, 18 Sep 2018 08:04:20 -0400
Subject: [PATCH] libvirt-domain.c:virDomainMigrateCheckNotLocal function
 return bool instead of int type

the function of virDomainMigrateCheckNotLocal return bool should be more in
line with specification,
and use return is better than goto.

Signed-off-by: xiajidong <xiajidong@cmss.chinamobile.com>
---
 src/libvirt-domain.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7690339..d741261 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3290,25 +3290,25 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
 }
 
 
-static int
+static bool
 virDomainMigrateCheckNotLocal(const char *dconnuri)
 {
     virURIPtr tempuri = NULL;
-    int ret = -1;
 
-    if (!(tempuri = virURIParse(dconnuri)))
-        goto cleanup;
+    if (!(tempuri = virURIParse(dconnuri))) {
+        virURIFree(tempuri);
+	     return false;
+    }
     if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) {
         virReportInvalidArg(dconnuri, "%s",
                             _("Attempt to migrate guest to the same
host"));
-        goto cleanup;
+        virURIFree(tempuri);
+	     return false;
     }
 
-    ret = 0;
 
- cleanup:
     virURIFree(tempuri);
-    return ret;
+    return ture;
 }
 
 
@@ -3428,7 +3428,7 @@ virDomainMigrateUnmanagedParams(virDomainPtr domain,
     VIR_TYPED_PARAMS_DEBUG(params, nparams);
 
     if ((flags & VIR_MIGRATE_PEER2PEER) &&
-        virDomainMigrateCheckNotLocal(dconnuri) < 0)
+        !virDomainMigrateCheckNotLocal(dconnuri))
         return -1;
 
     if ((flags & VIR_MIGRATE_PEER2PEER) &&
-- 
1.8.3.1




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Subject: [PATCH] libvirt-domain.c:virDomainMigrateCheckNotLocal function
Posted by Jiri Denemark 5 years, 7 months ago
On Tue, Sep 18, 2018 at 20:35:39 +0800, xiajidong@cmss.chinamobile.com wrote:
> >From c013b053d7514ee66b841bc99900b06d1e9d4dfd Mon Sep 17 00:00:00 2001
> From: xiajidong <xiajidong@cmss.chinamobile.com>
> Date: Tue, 18 Sep 2018 08:04:20 -0400
> Subject: [PATCH] libvirt-domain.c:virDomainMigrateCheckNotLocal function
>  return bool instead of int type
> 
> the function of virDomainMigrateCheckNotLocal return bool should be more in
> line with specification,
> and use return is better than goto.

Thanks for the contribution, but the patch is not really needed.

Libvirt functions and especially those which can report an error
generally return int: 0 for success, -1 for error. So the existing code
is consistent with this.

Using a cleanup section with goto is much better than repeating the same
cleanup code several times. Thus the current code is fine, while your
change is not right.

Your patch would not apply because it was sent incorrectly. Please,
consult https://libvirt.org/hacking.html before submitting patches for
libvirt.

And please, follow the coding style (the indentation is wrong in several
places) and make sure you run "make check syntax-check" before
submitting patches (the code would not compile, because of "return ture").

NACK

Jirka

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