1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemuMonitorJSONCheckReply: Use g_autofree

Yi Li posted 1 patch 3 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201016021733.3854335-1-yili@winhong.com
There is a newer version of this series
src/qemu/qemu_monitor_json.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
[PATCH] qemuMonitorJSONCheckReply: Use g_autofree
Posted by Yi Li 3 years, 6 months ago 
Eliminate cleanup code by using g_autofree.

Signed-off-by: Yi Li <yili@winhong.com>
---
 src/qemu/qemu_monitor_json.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 26ac499fc5..f76b369191 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -392,10 +392,11 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
                               virJSONValuePtr reply,
                               bool report)
 {
+    g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+    g_autofree char *replystr = virJSONValueToString(reply, false);
+
     if (virJSONValueObjectHasKey(reply, "error")) {
         virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
-        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-        g_autofree char *replystr = virJSONValueToString(reply, false);
 
         /* Log the full JSON formatted command & error */
         VIR_DEBUG("unable to execute QEMU command %s: %s",
@@ -417,8 +418,6 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
 
         return -1;
     } else if (!virJSONValueObjectHasKey(reply, "return")) {
-        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-        g_autofree char *replystr = virJSONValueToString(reply, false);
 
         VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s",
                   NULLSTR(cmdstr), NULLSTR(replystr));
@@ -455,8 +454,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 
     data = virJSONValueObjectGet(reply, "return");
     if (virJSONValueGetType(data) != type) {
-        char *cmdstr = virJSONValueToString(cmd, false);
-        char *retstr = virJSONValueToString(data, false);
+        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+        g_autofree char *retstr = virJSONValueToString(data, false);
 
         VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: %s",
                   virJSONValueGetType(data), type, cmdstr, retstr);
@@ -464,8 +463,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
                        _("unexpected type returned by QEMU command '%s'"),
                        qemuMonitorJSONCommandName(cmd));
 
-        VIR_FREE(cmdstr);
-        VIR_FREE(retstr);
         return -1;
     }
 
-- 
2.25.3
Re: [PATCH] qemuMonitorJSONCheckReply: Use g_autofree
Posted by Daniel Henrique Barboza 3 years, 6 months ago 
The commit title is a bit misleading because you're changing
qemuMonitorJSONCheckErrorFull() as well. I suggest changing it to:


"qemu_monitor_json.c: use g_autofree"

And then you cam mention the functions you're changing in the commit
message:


"Eliminate cleanup code by using g_autofree in qemuMonitorJSONCheckErrorFull()
and qemuMonitorJSONCheckReply()"


On 10/15/20 11:17 PM, Yi Li wrote:
> Eliminate cleanup code by using g_autofree.
> 
> Signed-off-by: Yi Li <yili@winhong.com>
> ---

The code LGTM. With the commit title/msg changes:


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


>   src/qemu/qemu_monitor_json.c | 13 +++++--------
>   1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 26ac499fc5..f76b369191 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -392,10 +392,11 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
>                                 virJSONValuePtr reply,
>                                 bool report)
>   {
> +    g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> +    g_autofree char *replystr = virJSONValueToString(reply, false);
> +
>       if (virJSONValueObjectHasKey(reply, "error")) {
>           virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
> -        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> -        g_autofree char *replystr = virJSONValueToString(reply, false);
>   
>           /* Log the full JSON formatted command & error */
>           VIR_DEBUG("unable to execute QEMU command %s: %s",
> @@ -417,8 +418,6 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
>   
>           return -1;
>       } else if (!virJSONValueObjectHasKey(reply, "return")) {
> -        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> -        g_autofree char *replystr = virJSONValueToString(reply, false);
>   
>           VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s",
>                     NULLSTR(cmdstr), NULLSTR(replystr));
> @@ -455,8 +454,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
>   
>       data = virJSONValueObjectGet(reply, "return");
>       if (virJSONValueGetType(data) != type) {
> -        char *cmdstr = virJSONValueToString(cmd, false);
> -        char *retstr = virJSONValueToString(data, false);
> +        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> +        g_autofree char *retstr = virJSONValueToString(data, false);
>   
>           VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: %s",
>                     virJSONValueGetType(data), type, cmdstr, retstr);
> @@ -464,8 +463,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
>                          _("unexpected type returned by QEMU command '%s'"),
>                          qemuMonitorJSONCommandName(cmd));
>   
> -        VIR_FREE(cmdstr);
> -        VIR_FREE(retstr);
>           return -1;
>       }
>   
>
[PATCH v2] qemu_monitor_json.c: use g_autofree
Posted by Yi Li 3 years, 6 months ago 
Eliminate cleanup code by using g_autofree in qemuMonitorJSONCheckErrorFull()
and qemuMonitorJSONCheckReply

Signed-off-by: Yi Li <yili@winhong.com>
---
 src/qemu/qemu_monitor_json.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 26ac499fc5..f76b369191 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -392,10 +392,11 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
                               virJSONValuePtr reply,
                               bool report)
 {
+    g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+    g_autofree char *replystr = virJSONValueToString(reply, false);
+
     if (virJSONValueObjectHasKey(reply, "error")) {
         virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
-        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-        g_autofree char *replystr = virJSONValueToString(reply, false);
 
         /* Log the full JSON formatted command & error */
         VIR_DEBUG("unable to execute QEMU command %s: %s",
@@ -417,8 +418,6 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
 
         return -1;
     } else if (!virJSONValueObjectHasKey(reply, "return")) {
-        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-        g_autofree char *replystr = virJSONValueToString(reply, false);
 
         VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s",
                   NULLSTR(cmdstr), NULLSTR(replystr));
@@ -455,8 +454,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 
     data = virJSONValueObjectGet(reply, "return");
     if (virJSONValueGetType(data) != type) {
-        char *cmdstr = virJSONValueToString(cmd, false);
-        char *retstr = virJSONValueToString(data, false);
+        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+        g_autofree char *retstr = virJSONValueToString(data, false);
 
         VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: %s",
                   virJSONValueGetType(data), type, cmdstr, retstr);
@@ -464,8 +463,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
                        _("unexpected type returned by QEMU command '%s'"),
                        qemuMonitorJSONCommandName(cmd));
 
-        VIR_FREE(cmdstr);
-        VIR_FREE(retstr);
         return -1;
     }
 
-- 
2.25.3
Re: [PATCH v2] qemu_monitor_json.c: use g_autofree
Posted by Peter Krempa 3 years, 6 months ago 
On Mon, Oct 19, 2020 at 09:17:44 +0800, Yi Li wrote:
> Eliminate cleanup code by using g_autofree in qemuMonitorJSONCheckErrorFull()
> and qemuMonitorJSONCheckReply
> 
> Signed-off-by: Yi Li <yili@winhong.com>
> ---
>  src/qemu/qemu_monitor_json.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 26ac499fc5..f76b369191 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -392,10 +392,11 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
>                                virJSONValuePtr reply,
>                                bool report)
>  {
> +    g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> +    g_autofree char *replystr = virJSONValueToString(reply, false);
> +
>      if (virJSONValueObjectHasKey(reply, "error")) {
>          virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
> -        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> -        g_autofree char *replystr = virJSONValueToString(reply, false);
>  
>          /* Log the full JSON formatted command & error */
>          VIR_DEBUG("unable to execute QEMU command %s: %s",
> @@ -417,8 +418,6 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
>  
>          return -1;
>      } else if (!virJSONValueObjectHasKey(reply, "return")) {
> -        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> -        g_autofree char *replystr = virJSONValueToString(reply, false);
>  
>          VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s",
>                    NULLSTR(cmdstr), NULLSTR(replystr));

These two hunks don't make sense in context of the patch, both use
autofree. The patch only changes scope/duplication of the variables.

If you want to do this change it should be separate with separate
justification.

> @@ -455,8 +454,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
>  
>      data = virJSONValueObjectGet(reply, "return");
>      if (virJSONValueGetType(data) != type) {
> -        char *cmdstr = virJSONValueToString(cmd, false);
> -        char *retstr = virJSONValueToString(data, false);
> +        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
> +        g_autofree char *retstr = virJSONValueToString(data, false);
>  
>          VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: %s",
>                    virJSONValueGetType(data), type, cmdstr, retstr);
> @@ -464,8 +463,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
>                         _("unexpected type returned by QEMU command '%s'"),
>                         qemuMonitorJSONCommandName(cmd));
>  
> -        VIR_FREE(cmdstr);
> -        VIR_FREE(retstr);
>          return -1;
>      }

These hunks are okay.
[PATCH v3] qemuMonitorJSONCheckReply: Use g_autofree
Posted by Yi Li 3 years, 6 months ago 
Eliminate cleanup code by using g_autofree.

Signed-off-by: Yi Li <yili@winhong.com>
---
 src/qemu/qemu_monitor_json.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 6c763ecc12..4994ace071 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -455,8 +455,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 
     data = virJSONValueObjectGet(reply, "return");
     if (virJSONValueGetType(data) != type) {
-        char *cmdstr = virJSONValueToString(cmd, false);
-        char *retstr = virJSONValueToString(data, false);
+        g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+        g_autofree char *retstr = virJSONValueToString(data, false);
 
         VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: %s",
                   virJSONValueGetType(data), type, cmdstr, retstr);
@@ -464,8 +464,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
                        _("unexpected type returned by QEMU command '%s'"),
                        qemuMonitorJSONCommandName(cmd));
 
-        VIR_FREE(cmdstr);
-        VIR_FREE(retstr);
         return -1;
     }
 
-- 
2.25.3
Re: [PATCH v3] qemuMonitorJSONCheckReply: Use g_autofree
Posted by Ján Tomko 3 years, 5 months ago 
On a Wednesday in 2020, Yi Li wrote:
>Eliminate cleanup code by using g_autofree.
>
>Signed-off-by: Yi Li <yili@winhong.com>
>---
> src/qemu/qemu_monitor_json.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

And pushed.

Jano

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.