Re: [libvirt PATCH 5/9] qemu: use virJSONValueObjectGetStringArray

2020-11-30 Thread Marc-André Lureau
On Mon, Nov 30, 2020 at 11:16 PM Michal Privoznik  wrote:
>
> On 11/20/20 7:09 PM, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > There might be more potential users around, I haven't looked thoroughly.
>
>
> Quick git grep showed two more: qemuAgentSSHGetAuthorizedKeys() and
> qemuMonitorJSONGetStringListProperty(). But that can be done in a
> separate patch.
>
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >   src/qemu/qemu_monitor_json.c | 34 ++
> >   1 file changed, 6 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> > index 723bdb4426..3588a0c426 100644
> > --- a/src/qemu/qemu_monitor_json.c
> > +++ b/src/qemu/qemu_monitor_json.c
> > @@ -7533,10 +7533,7 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, 
> > const char *qmpCmd,
> >   int ret = -1;
> >   virJSONValuePtr cmd;
> >   virJSONValuePtr reply = NULL;
> > -virJSONValuePtr data;
> > -char **list = NULL;
> > -size_t n = 0;
> > -size_t i;
> > +g_auto(GStrv) list = NULL;
> >
> >   *array = NULL;
> >
> > @@ -7554,32 +7551,13 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, 
> > const char *qmpCmd,
> >   if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
> >   goto cleanup;
> >
> > -data = virJSONValueObjectGetArray(reply, "return");
> > -n = virJSONValueArraySize(data);
> > -
> > -/* null-terminated list */
> > -list = g_new0(char *, n + 1);
> > -
> > -for (i = 0; i < n; i++) {
> > -virJSONValuePtr child = virJSONValueArrayGet(data, i);
> > -const char *tmp;
> > -
> > -if (!(tmp = virJSONValueGetString(child))) {
> > -virReportError(VIR_ERR_INTERNAL_ERROR,
> > -   _("%s array element does not contain data"),
> > -   qmpCmd);
> > -goto cleanup;
> > -}
> > -
> > -list[i] = g_strdup(tmp);
> > -}
> > -
> > -ret = n;
> > -*array = list;
> > -list = NULL;
> > +list = virJSONValueObjectGetStringArray(reply, "return");
>
> We can ditch the @list variable and assign to *array directly.
>
> > +if (!list)
> > +goto cleanup;
> > +ret = g_strv_length(list);
> > +*array = g_steal_pointer();
> >
> >cleanup:
> > -g_strfreev(list);
> >   virJSONValueFree(cmd);
> >   virJSONValueFree(reply);
> >   return ret;
> >
>
>
> How about this squashed in:
>
>
> diff --git c/src/qemu/qemu_monitor_json.c i/src/qemu/qemu_monitor_json.c
> index 3588a0c426..e7aa6b6ffd 100644
> --- c/src/qemu/qemu_monitor_json.c
> +++ i/src/qemu/qemu_monitor_json.c
> @@ -7533,7 +7533,6 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon,
> const char *qmpCmd,
>   int ret = -1;
>   virJSONValuePtr cmd;
>   virJSONValuePtr reply = NULL;
> -g_auto(GStrv) list = NULL;
>
>   *array = NULL;
>
> @@ -7551,11 +7550,10 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr
> mon, const char *qmpCmd,
>   if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
>   goto cleanup;
>
> -list = virJSONValueObjectGetStringArray(reply, "return");
> -if (!list)
> +if (!(*array = virJSONValueObjectGetStringArray(reply, "return")))
>   goto cleanup;
> -ret = g_strv_length(list);
> -*array = g_steal_pointer();
> +
> +ret = g_strv_length(*array);
>
>cleanup:
>   virJSONValueFree(cmd);
>

Looks good to me, thanks




Re: [libvirt PATCH 5/9] qemu: use virJSONValueObjectGetStringArray

2020-11-30 Thread Michal Privoznik

On 11/20/20 7:09 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

There might be more potential users around, I haven't looked thoroughly.



Quick git grep showed two more: qemuAgentSSHGetAuthorizedKeys() and 
qemuMonitorJSONGetStringListProperty(). But that can be done in a 
separate patch.




Signed-off-by: Marc-André Lureau 
---
  src/qemu/qemu_monitor_json.c | 34 ++
  1 file changed, 6 insertions(+), 28 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 723bdb4426..3588a0c426 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7533,10 +7533,7 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const 
char *qmpCmd,
  int ret = -1;
  virJSONValuePtr cmd;
  virJSONValuePtr reply = NULL;
-virJSONValuePtr data;
-char **list = NULL;
-size_t n = 0;
-size_t i;
+g_auto(GStrv) list = NULL;
  
  *array = NULL;
  
@@ -7554,32 +7551,13 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd,

  if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
  goto cleanup;
  
-data = virJSONValueObjectGetArray(reply, "return");

-n = virJSONValueArraySize(data);
-
-/* null-terminated list */
-list = g_new0(char *, n + 1);
-
-for (i = 0; i < n; i++) {
-virJSONValuePtr child = virJSONValueArrayGet(data, i);
-const char *tmp;
-
-if (!(tmp = virJSONValueGetString(child))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("%s array element does not contain data"),
-   qmpCmd);
-goto cleanup;
-}
-
-list[i] = g_strdup(tmp);
-}
-
-ret = n;
-*array = list;
-list = NULL;
+list = virJSONValueObjectGetStringArray(reply, "return");


We can ditch the @list variable and assign to *array directly.


+if (!list)
+goto cleanup;
+ret = g_strv_length(list);
+*array = g_steal_pointer();
  
   cleanup:

-g_strfreev(list);
  virJSONValueFree(cmd);
  virJSONValueFree(reply);
  return ret;




How about this squashed in:


diff --git c/src/qemu/qemu_monitor_json.c i/src/qemu/qemu_monitor_json.c
index 3588a0c426..e7aa6b6ffd 100644
--- c/src/qemu/qemu_monitor_json.c
+++ i/src/qemu/qemu_monitor_json.c
@@ -7533,7 +7533,6 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, 
const char *qmpCmd,

 int ret = -1;
 virJSONValuePtr cmd;
 virJSONValuePtr reply = NULL;
-g_auto(GStrv) list = NULL;

 *array = NULL;

@@ -7551,11 +7550,10 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr 
mon, const char *qmpCmd,

 if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
 goto cleanup;

-list = virJSONValueObjectGetStringArray(reply, "return");
-if (!list)
+if (!(*array = virJSONValueObjectGetStringArray(reply, "return")))
 goto cleanup;
-ret = g_strv_length(list);
-*array = g_steal_pointer();
+
+ret = g_strv_length(*array);

  cleanup:
 virJSONValueFree(cmd);


Michal