6:55 p.m.
Peter Krempa (25):
virLockDaemonPreExecRestart: Refactor memory cleanup
virLogDaemonPreExecRestart: Refactor memory cleanup
virLogHandlerPreExecRestart: Refactor memory cleanup
virNetDaemonPreExecRestart: Refactor memory cleanup
virNetServerServicePreExecRestart: Refactor memory cleanup
virNetServerClientPreExecRestart: Refactor memory cleanup
virNetServerPreExecRestart: Drop error reporting from
virJSONValueObjectAppend* calls
virNetServerPreExecRestart: Refactor memory cleanup
virLockSpacePreExecRestart: Refactor memory cleanup
qemuAgentMakeCommand: Refactor memory cleanup
virJSONValueObjectInsert: Clear @value on successful insertion
virJSONValueCopy: Don't use virJSONValue(Object|Array)Append
virJSONValue(Array|Object)Append*: Simplify handling of appended
object
virJSONValueNewArrayFromBitmap: Refactor cleanup
virJSONValueObjectAddVArgs: Use autofree for the temporary bitmap
virJSONValueObjectAppend: Clear pointer when taking ownership of
passed value
qemuAgentMakeStringsArray: Refactor cleanup
virMACMapHashDumper: Refactor array addition
testQEMUSchemaValidateObjectMergeVariantMember: Fix theoretical leak
virJSONValueArrayAppend: Clear pointer when taking ownership of passed
value
qemuMonitorJSONTransactionAdd: Refactor cleanup
qemuAgentSetVCPUsCommand: Refactor cleanup
virJSONParserHandle*: Refactor memory cleanup and drop NULL checks
virJSONValueNewNumber: Take ownership of passed string
virJSONParserInsertValue: Take double pointer for @value
src/locking/lock_daemon.c | 77 ++++----
src/logging/log_daemon.c | 52 +++---
src/logging/log_handler.c | 40 ++--
src/network/leaseshelper.c | 3 +-
src/node_device/node_device_driver.c | 4 +-
src/qemu/qemu_agent.c | 89 ++++-----
src/qemu/qemu_block.c | 19 +-
src/qemu/qemu_command.c | 3 +-
src/qemu/qemu_firmware.c | 27 +--
src/qemu/qemu_migration_params.c | 4 +-
src/qemu/qemu_monitor_json.c | 58 +++---
src/rpc/virnetdaemon.c | 25 +--
src/rpc/virnetserver.c | 79 +++-----
src/rpc/virnetserverclient.c | 24 +--
src/rpc/virnetserverservice.c | 34 ++--
src/util/virjson.c | 261 +++++++++++----------------
src/util/virjson.h | 7 +-
src/util/virlease.c | 2 +-
src/util/virlockspace.c | 47 ++---
src/util/virmacmap.c | 13 +-
tests/testutilsqemuschema.c | 4 +-
21 files changed, 322 insertions(+), 550 deletions(-)
--
2.29.2
6:55 p.m.
New subject: [PATCH 01/25] virLockDaemonPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/locking/lock_daemon.c | 81 +++++++++++++++++----------------------
1 file changed, 35 insertions(+), 46 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 94fe374df6..5913c0cb9c 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -697,87 +697,76 @@ virLockDaemonPreExecRestart(const char *state_file,
virNetDaemonPtr dmn,
char **argv)
{
- virJSONValuePtr child;
- char *state = NULL;
- virJSONValuePtr object = virJSONValueNewObject();
- char *magic = NULL;
- virHashKeyValuePairPtr pairs = NULL, tmp;
- virJSONValuePtr lockspaces;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) lockspaces = virJSONValueNewArray();
+ g_autoptr(virJSONValue) defaultLockspace = NULL;
+ g_autoptr(virJSONValue) daemon = NULL;
+ g_autofree char *state = NULL;
+ g_autofree char *magic = NULL;
+ g_autofree virHashKeyValuePairPtr pairs = NULL;
+ virHashKeyValuePairPtr tmp;
VIR_DEBUG("Running pre-restart exec");
- if (!(child = virNetDaemonPreExecRestart(dmn)))
- goto cleanup;
-
- if (virJSONValueObjectAppend(object, "daemon", child) < 0) {
- virJSONValueFree(child);
- goto cleanup;
- }
-
- if (!(child = virLockSpacePreExecRestart(lockDaemon->defaultLockspace)))
- goto cleanup;
-
- if (virJSONValueObjectAppend(object, "defaultLockspace", child) < 0) {
- virJSONValueFree(child);
- goto cleanup;
- }
+ if (!(daemon = virNetDaemonPreExecRestart(dmn)))
+ return -1;
- lockspaces = virJSONValueNewArray();
+ if (virJSONValueObjectAppend(object, "daemon", daemon) < 0)
+ return -1;
+ daemon = NULL;
- if (virJSONValueObjectAppend(object, "lockspaces", lockspaces) < 0) {
- virJSONValueFree(lockspaces);
- goto cleanup;
- }
+ if (!(defaultLockspace =
virLockSpacePreExecRestart(lockDaemon->defaultLockspace)))
+ return -1;
+ if (virJSONValueObjectAppend(object, "defaultLockspace", defaultLockspace)
< 0)
+ return -1;
+ defaultLockspace = NULL;
tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL, false);
while (tmp && tmp->key) {
virLockSpacePtr lockspace = (virLockSpacePtr)tmp->value;
+ g_autoptr(virJSONValue) child = NULL;
if (!(child = virLockSpacePreExecRestart(lockspace)))
- goto cleanup;
+ return -1;
- if (virJSONValueArrayAppend(lockspaces, child) < 0) {
- virJSONValueFree(child);
- goto cleanup;
- }
+ if (virJSONValueArrayAppend(lockspaces, child) < 0)
+ return -1;
+ child = NULL;
tmp++;
}
+ if (virJSONValueObjectAppend(object, "lockspaces", lockspaces) < 0)
+ return -1;
+ lockspaces = NULL;
+
if (!(magic = virLockDaemonGetExecRestartMagic()))
- goto cleanup;
+ return -1;
if (virJSONValueObjectAppendString(object, "magic", magic) < 0)
- goto cleanup;
+ return -1;
if (!(state = virJSONValueToString(object, true)))
- goto cleanup;
+ return -1;
VIR_DEBUG("Saving state %s", state);
- if (virFileWriteStr(state_file,
- state, 0700) < 0) {
+ if (virFileWriteStr(state_file, state, 0700) < 0) {
virReportSystemError(errno,
- _("Unable to save state file %s"),
- state_file);
- goto cleanup;
+ _("Unable to save state file %s"), state_file);
+ return -1;
}
if (execvp(argv[0], argv) < 0) {
virReportSystemError(errno, "%s",
_("Unable to restart self"));
- goto cleanup;
+ return -1;
}
abort(); /* This should be impossible to reach */
- cleanup:
- VIR_FREE(magic);
- VIR_FREE(pairs);
- VIR_FREE(state);
- virJSONValueFree(object);
- return -1;
+ return 0;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 02/25] virLogDaemonPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/logging/log_daemon.c | 54 +++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 770f6dd273..ceffa6a368 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -505,62 +505,54 @@ virLogDaemonPreExecRestart(const char *state_file,
virNetDaemonPtr dmn,
char **argv)
{
- virJSONValuePtr child;
- char *state = NULL;
- virJSONValuePtr object = virJSONValueNewObject();
- char *magic = NULL;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) daemon = NULL;
+ g_autoptr(virJSONValue) handler = NULL;
+ g_autofree char *state = NULL;
+ g_autofree char *magic = NULL;
VIR_DEBUG("Running pre-restart exec");
- if (!(child = virNetDaemonPreExecRestart(dmn)))
- goto cleanup;
+ if (!(daemon = virNetDaemonPreExecRestart(dmn)))
+ return -1;
- if (virJSONValueObjectAppend(object, "daemon", child) < 0) {
- virJSONValueFree(child);
- goto cleanup;
- }
+ if (virJSONValueObjectAppend(object, "daemon", daemon) < 0)
+ return -1;
+ daemon = NULL;
if (!(magic = virLogDaemonGetExecRestartMagic()))
- goto cleanup;
+ return -1;
if (virJSONValueObjectAppendString(object, "magic", magic) < 0)
- goto cleanup;
-
- if (!(child = virLogHandlerPreExecRestart(logDaemon->handler)))
- goto cleanup;
+ return -1;
- if (virJSONValueObjectAppend(object, "handler", child) < 0) {
- virJSONValueFree(child);
- goto cleanup;
- }
+ if (!(handler = virLogHandlerPreExecRestart(logDaemon->handler)))
+ return -1;
+ if (virJSONValueObjectAppend(object, "handler", handler) < 0)
+ return -1;
+ handler = NULL;
if (!(state = virJSONValueToString(object, true)))
- goto cleanup;
+ return -1;
VIR_DEBUG("Saving state %s", state);
- if (virFileWriteStr(state_file,
- state, 0700) < 0) {
+ if (virFileWriteStr(state_file, state, 0700) < 0) {
virReportSystemError(errno,
- _("Unable to save state file %s"),
- state_file);
- goto cleanup;
+ _("Unable to save state file %s"), state_file);
+ return -1;
}
if (execvp(argv[0], argv) < 0) {
virReportSystemError(errno, "%s",
_("Unable to restart self"));
- goto cleanup;
+ return -1;
}
abort(); /* This should be impossible to reach */
- cleanup:
- VIR_FREE(magic);
- VIR_FREE(state);
- virJSONValueFree(object);
- return -1;
+ return 0;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 03/25] virLogHandlerPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/logging/log_handler.c | 42 ++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index a77c1e0250..cacf9584cd 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -608,56 +608,48 @@ virLogHandlerDomainAppendLogFile(virLogHandlerPtr handler,
virJSONValuePtr
virLogHandlerPreExecRestart(virLogHandlerPtr handler)
{
- virJSONValuePtr ret = virJSONValueNewObject();
- virJSONValuePtr files;
+ g_autoptr(virJSONValue) ret = virJSONValueNewObject();
+ g_autoptr(virJSONValue) files = virJSONValueNewArray();
size_t i;
char domuuid[VIR_UUID_STRING_BUFLEN];
- files = virJSONValueNewArray();
-
- if (virJSONValueObjectAppend(ret, "files", files) < 0) {
- virJSONValueFree(files);
- goto error;
- }
-
for (i = 0; i < handler->nfiles; i++) {
- virJSONValuePtr file = virJSONValueNewObject();
-
- if (virJSONValueArrayAppend(files, file) < 0) {
- virJSONValueFree(file);
- goto error;
- }
+ g_autoptr(virJSONValue) file = virJSONValueNewObject();
if (virJSONValueObjectAppendNumberInt(file, "pipefd",
handler->files[i]->pipefd) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendString(file, "path",
virRotatingFileWriterGetPath(handler->files[i]->file)) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendString(file, "driver",
handler->files[i]->driver) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendString(file, "domname",
handler->files[i]->domname) < 0)
- goto error;
+ return NULL;
virUUIDFormat(handler->files[i]->domuuid, domuuid);
if (virJSONValueObjectAppendString(file, "domuuid", domuuid) < 0)
- goto error;
+ return NULL;
if (virSetInherit(handler->files[i]->pipefd, true) < 0) {
virReportSystemError(errno, "%s",
_("Cannot disable close-on-exec flag"));
- goto error;
+ return NULL;
}
+
+ if (virJSONValueArrayAppend(files, file) < 0)
+ return NULL;
+ file = NULL;
}
- return ret;
+ if (virJSONValueObjectAppend(ret, "files", files) < 0)
+ return NULL;
+ files = NULL;
- error:
- virJSONValueFree(ret);
- return NULL;
+ return g_steal_pointer(&ret);
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 04/25] virNetDaemonPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/rpc/virnetdaemon.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 2c18da790b..6132615093 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -384,23 +384,18 @@ virJSONValuePtr
virNetDaemonPreExecRestart(virNetDaemonPtr dmn)
{
size_t i = 0;
- virJSONValuePtr object = virJSONValueNewObject();
- virJSONValuePtr srvObj = virJSONValueNewObject();
- virHashKeyValuePairPtr srvArray = NULL;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) srvObj = virJSONValueNewObject();
+ g_autofree virHashKeyValuePairPtr srvArray = NULL;
virObjectLock(dmn);
- if (virJSONValueObjectAppend(object, "servers", srvObj) < 0) {
- virJSONValueFree(srvObj);
- goto error;
- }
-
if (!(srvArray = virHashGetItems(dmn->servers, NULL, true)))
goto error;
for (i = 0; srvArray[i].key; i++) {
virNetServerPtr server = virHashLookup(dmn->servers, srvArray[i].key);
- virJSONValuePtr srvJSON;
+ g_autoptr(virJSONValue) srvJSON = NULL;
if (!server)
goto error;
@@ -409,20 +404,20 @@ virNetDaemonPreExecRestart(virNetDaemonPtr dmn)
if (!srvJSON)
goto error;
- if (virJSONValueObjectAppend(srvObj, srvArray[i].key, srvJSON) < 0) {
- virJSONValueFree(srvJSON);
+ if (virJSONValueObjectAppend(srvObj, srvArray[i].key, srvJSON) < 0)
goto error;
- }
+ srvJSON = NULL;
}
- VIR_FREE(srvArray);
virObjectUnlock(dmn);
- return object;
+ if (virJSONValueObjectAppend(object, "servers", srvObj) < 0)
+ return NULL;
+ srvObj = NULL;
+
+ return g_steal_pointer(&object);
error:
- VIR_FREE(srvArray);
- virJSONValueFree(object);
virObjectUnlock(dmn);
return NULL;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 05/25] virNetServerServicePreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/rpc/virnetserverservice.c | 36 ++++++++++++++---------------------
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 73232e3747..a72277226a 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -336,40 +336,32 @@ virNetServerServicePtr
virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
virJSONValuePtr virNetServerServicePreExecRestart(virNetServerServicePtr svc)
{
- virJSONValuePtr object = virJSONValueNewObject();
- virJSONValuePtr socks;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) socks = virJSONValueNewArray();
size_t i;
if (virJSONValueObjectAppendNumberInt(object, "auth", svc->auth) <
0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendBoolean(object, "readonly", svc->readonly)
< 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendNumberUint(object, "nrequests_client_max",
svc->nrequests_client_max) < 0)
- goto error;
-
- socks = virJSONValueNewArray();
-
- if (virJSONValueObjectAppend(object, "socks", socks) < 0) {
- virJSONValueFree(socks);
- goto error;
- }
+ return NULL;
for (i = 0; i < svc->nsocks; i++) {
- virJSONValuePtr child;
+ g_autoptr(virJSONValue) child = NULL;
if (!(child = virNetSocketPreExecRestart(svc->socks[i])))
- goto error;
+ return NULL;
- if (virJSONValueArrayAppend(socks, child) < 0) {
- virJSONValueFree(child);
- goto error;
- }
+ if (virJSONValueArrayAppend(socks, child) < 0)
+ return NULL;
+ child = NULL;
}
- return object;
+ if (virJSONValueObjectAppend(object, "socks", socks) < 0)
+ return NULL;
+ socks = NULL;
- error:
- virJSONValueFree(object);
- return NULL;
+ return g_steal_pointer(&object);
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 06/25] virNetServerClientPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/rpc/virnetserverclient.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 4d01e87e21..0789ad9154 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -585,15 +585,14 @@ virNetServerClientPtr
virNetServerClientNewPostExecRestart(virNetServerPtr srv,
virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client)
{
- virJSONValuePtr object = virJSONValueNewObject();
- virJSONValuePtr child;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) sock = NULL;
+ g_autoptr(virJSONValue) priv = NULL;
virObjectLock(client);
- if (virJSONValueObjectAppendNumberUlong(object, "id",
- client->id) < 0)
+ if (virJSONValueObjectAppendNumberUlong(object, "id", client->id) <
0)
goto error;
-
if (virJSONValueObjectAppendNumberInt(object, "auth", client->auth) <
0)
goto error;
if (virJSONValueObjectAppendBoolean(object, "auth_pending",
client->auth_pending) < 0)
@@ -608,28 +607,25 @@ virJSONValuePtr
virNetServerClientPreExecRestart(virNetServerClientPtr client)
client->conn_time) < 0)
goto error;
- if (!(child = virNetSocketPreExecRestart(client->sock)))
+ if (!(sock = virNetSocketPreExecRestart(client->sock)))
goto error;
- if (virJSONValueObjectAppend(object, "sock", child) < 0) {
- virJSONValueFree(child);
+ if (virJSONValueObjectAppend(object, "sock", sock) < 0)
goto error;
- }
+ sock = NULL;
- if (!(child = client->privateDataPreExecRestart(client, client->privateData)))
+ if (!(priv = client->privateDataPreExecRestart(client, client->privateData)))
goto error;
- if (virJSONValueObjectAppend(object, "privateData", child) < 0) {
- virJSONValueFree(child);
+ if (virJSONValueObjectAppend(object, "privateData", priv) < 0)
goto error;
- }
+ priv = NULL;
virObjectUnlock(client);
- return object;
+ return g_steal_pointer(&object);
error:
virObjectUnlock(client);
- virJSONValueFree(object);
return NULL;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 07/25] virNetServerPreExecRestart: Drop error reporting from virJSONValueObjectAppend* calls
The functions report errors already and the error can nowadays only
happen on programmer errors (if the passed virJSONValue isn't an
object), which won't happen. Remove the reporting.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/rpc/virnetserver.c | 42 ++++++++++--------------------------------
1 file changed, 10 insertions(+), 32 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index f0b866a962..ab8dafb1bb 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -556,51 +556,29 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
virObjectLock(srv);
if (virJSONValueObjectAppendNumberUint(object, "min_workers",
- virThreadPoolGetMinWorkers(srv->workers))
< 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set min_workers data in JSON document"));
+ virThreadPoolGetMinWorkers(srv->workers))
< 0)
goto error;
- }
if (virJSONValueObjectAppendNumberUint(object, "max_workers",
- virThreadPoolGetMaxWorkers(srv->workers))
< 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set max_workers data in JSON document"));
+ virThreadPoolGetMaxWorkers(srv->workers))
< 0)
goto error;
- }
if (virJSONValueObjectAppendNumberUint(object, "priority_workers",
-
virThreadPoolGetPriorityWorkers(srv->workers)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set priority_workers data in JSON
document"));
+
virThreadPoolGetPriorityWorkers(srv->workers)) < 0)
goto error;
- }
- if (virJSONValueObjectAppendNumberUint(object, "max_clients",
srv->nclients_max) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set max_clients data in JSON document"));
+
+ if (virJSONValueObjectAppendNumberUint(object, "max_clients",
srv->nclients_max) < 0)
goto error;
- }
if (virJSONValueObjectAppendNumberUint(object, "max_anonymous_clients",
- srv->nclients_unauth_max) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set max_anonymous_clients data in JSON
document"));
+ srv->nclients_unauth_max) < 0)
goto error;
- }
- if (virJSONValueObjectAppendNumberUint(object, "keepaliveInterval",
srv->keepaliveInterval) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set keepaliveInterval data in JSON
document"));
+
+ if (virJSONValueObjectAppendNumberUint(object, "keepaliveInterval",
srv->keepaliveInterval) < 0)
goto error;
- }
- if (virJSONValueObjectAppendNumberUint(object, "keepaliveCount",
srv->keepaliveCount) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set keepaliveCount data in JSON document"));
+ if (virJSONValueObjectAppendNumberUint(object, "keepaliveCount",
srv->keepaliveCount) < 0)
goto error;
- }
if (virJSONValueObjectAppendNumberUlong(object, "next_client_id",
- srv->next_client_id) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot set next_client_id data in JSON document"));
+ srv->next_client_id) < 0)
goto error;
- }
services = virJSONValueNewArray();
--
2.29.2
6:55 p.m.
New subject: [PATCH 08/25] virNetServerPreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/rpc/virnetserver.c | 41 ++++++++++++++++-------------------------
1 file changed, 16 insertions(+), 25 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index ab8dafb1bb..ee402e8ea0 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -548,9 +548,9 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr
object,
virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
{
- virJSONValuePtr object = virJSONValueNewObject();
- virJSONValuePtr clients;
- virJSONValuePtr services;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) clients = virJSONValueNewArray();
+ g_autoptr(virJSONValue) services = virJSONValueNewArray();
size_t i;
virObjectLock(srv);
@@ -580,48 +580,39 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
srv->next_client_id) < 0)
goto error;
- services = virJSONValueNewArray();
-
- if (virJSONValueObjectAppend(object, "services", services) < 0) {
- virJSONValueFree(services);
- goto error;
- }
-
for (i = 0; i < srv->nservices; i++) {
- virJSONValuePtr child;
+ g_autoptr(virJSONValue) child = NULL;
if (!(child = virNetServerServicePreExecRestart(srv->services[i])))
goto error;
- if (virJSONValueArrayAppend(services, child) < 0) {
- virJSONValueFree(child);
+ if (virJSONValueArrayAppend(services, child) < 0)
goto error;
- }
+ child = NULL;
}
- clients = virJSONValueNewArray();
-
- if (virJSONValueObjectAppend(object, "clients", clients) < 0) {
- virJSONValueFree(clients);
+ if (virJSONValueObjectAppend(object, "services", services) < 0)
goto error;
- }
+ services = NULL;
for (i = 0; i < srv->nclients; i++) {
- virJSONValuePtr child;
+ g_autoptr(virJSONValue) child = NULL;
if (!(child = virNetServerClientPreExecRestart(srv->clients[i])))
goto error;
- if (virJSONValueArrayAppend(clients, child) < 0) {
- virJSONValueFree(child);
+ if (virJSONValueArrayAppend(clients, child) < 0)
goto error;
- }
+ child = NULL;
}
+ if (virJSONValueObjectAppend(object, "clients", clients) < 0)
+ goto error;
+ clients = NULL;
+
virObjectUnlock(srv);
- return object;
+ return g_steal_pointer(&object);
error:
- virJSONValueFree(object);
virObjectUnlock(srv);
return NULL;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 09/25] virLockSpacePreExecRestart: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virlockspace.c | 51 +++++++++++++++++------------------------
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 7df319004e..1b6b51b649 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -418,9 +418,10 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr
object)
virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
{
- virJSONValuePtr object = virJSONValueNewObject();
- virJSONValuePtr resources;
- virHashKeyValuePairPtr pairs = NULL, tmp;
+ g_autoptr(virJSONValue) object = virJSONValueNewObject();
+ g_autoptr(virJSONValue) resources = virJSONValueNewArray();
+ g_autofree virHashKeyValuePairPtr pairs = NULL;
+ virHashKeyValuePairPtr tmp;
virMutexLock(&lockspace->lock);
@@ -428,25 +429,14 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr
lockspace)
virJSONValueObjectAppendString(object, "directory", lockspace->dir)
< 0)
goto error;
- resources = virJSONValueNewArray();
-
- if (virJSONValueObjectAppend(object, "resources", resources) < 0) {
- virJSONValueFree(resources);
- goto error;
- }
tmp = pairs = virHashGetItems(lockspace->resources, NULL, false);
while (tmp && tmp->value) {
virLockSpaceResourcePtr res = (virLockSpaceResourcePtr)tmp->value;
- virJSONValuePtr child = virJSONValueNewObject();
- virJSONValuePtr owners = NULL;
+ g_autoptr(virJSONValue) child = virJSONValueNewObject();
+ g_autoptr(virJSONValue) owners = virJSONValueNewArray();
size_t i;
- if (virJSONValueArrayAppend(resources, child) < 0) {
- virJSONValueFree(child);
- goto error;
- }
-
if (virJSONValueObjectAppendString(child, "name", res->name) < 0
||
virJSONValueObjectAppendString(child, "path", res->path) < 0
||
virJSONValueObjectAppendNumberInt(child, "fd", res->fd) < 0
||
@@ -460,34 +450,35 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr
lockspace)
goto error;
}
- owners = virJSONValueNewArray();
-
- if (virJSONValueObjectAppend(child, "owners", owners) < 0) {
- virJSONValueFree(owners);
- goto error;
- }
-
for (i = 0; i < res->nOwners; i++) {
- virJSONValuePtr owner = virJSONValueNewNumberUlong(res->owners[i]);
+ g_autoptr(virJSONValue) owner =
virJSONValueNewNumberUlong(res->owners[i]);
if (!owner)
goto error;
- if (virJSONValueArrayAppend(owners, owner) < 0) {
- virJSONValueFree(owner);
+ if (virJSONValueArrayAppend(owners, owner) < 0)
goto error;
- }
+ owner = NULL;
}
+ if (virJSONValueObjectAppend(child, "owners", owners) < 0)
+ goto error;
+ owners = NULL;
+
+ if (virJSONValueArrayAppend(resources, child) < 0)
+ goto error;
+ child = NULL;
+
tmp++;
}
- VIR_FREE(pairs);
+
+ if (virJSONValueObjectAppend(object, "resources", resources) < 0)
+ goto error;
+ resources = NULL;
virMutexUnlock(&lockspace->lock);
return object;
error:
- VIR_FREE(pairs);
- virJSONValueFree(object);
virMutexUnlock(&lockspace->lock);
return NULL;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 10/25] qemuAgentMakeCommand: Refactor memory cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_agent.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 51cc00c618..4712aeb529 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1149,31 +1149,26 @@ static virJSONValuePtr G_GNUC_NULL_TERMINATED
qemuAgentMakeCommand(const char *cmdname,
...)
{
- virJSONValuePtr obj = virJSONValueNewObject();
- virJSONValuePtr jargs = NULL;
+ g_autoptr(virJSONValue) obj = NULL;
+ g_autoptr(virJSONValue) jargs = NULL;
va_list args;
va_start(args, cmdname);
- if (virJSONValueObjectAppendString(obj, "execute", cmdname) < 0)
- goto error;
-
- if (virJSONValueObjectCreateVArgs(&jargs, args) < 0)
- goto error;
-
- if (jargs &&
- virJSONValueObjectAppend(obj, "arguments", jargs) < 0)
- goto error;
+ if (virJSONValueObjectCreateVArgs(&jargs, args) < 0) {
+ va_end(args);
+ return NULL;
+ }
va_end(args);
- return obj;
+ if (virJSONValueObjectCreate(&obj,
+ "s:execute", cmdname,
+ "A:arguments", &jargs,
+ NULL) < 0)
+ return NULL;
- error:
- virJSONValueFree(obj);
- virJSONValueFree(jargs);
- va_end(args);
- return NULL;
+ return g_steal_pointer(&obj);
}
static virJSONValuePtr
--
2.29.2
6:55 p.m.
New subject: [PATCH 11/25] virJSONValueObjectInsert: Clear @value on successful insertion
The function takes ownership of @value on success so the proper
semantics will be to clear out the @value pointer. Convert @value to a
double pointer to do this.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index a509943fde..80ebcb587c 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -577,10 +577,10 @@ virJSONValueNewObject(void)
static int
virJSONValueObjectInsert(virJSONValuePtr object,
const char *key,
- virJSONValuePtr value,
+ virJSONValuePtr *value,
bool prepend)
{
- virJSONObjectPair pair = { NULL, value };
+ virJSONObjectPair pair = { NULL, *value };
int ret = -1;
if (object->type != VIR_JSON_TYPE_OBJECT) {
@@ -604,6 +604,9 @@ virJSONValueObjectInsert(virJSONValuePtr object,
object->data.object.npairs, pair);
}
+ if (ret == 0)
+ *value = NULL;
+
VIR_FREE(pair.key);
return ret;
}
@@ -614,7 +617,7 @@ virJSONValueObjectAppend(virJSONValuePtr object,
const char *key,
virJSONValuePtr value)
{
- return virJSONValueObjectInsert(object, key, value, false);
+ return virJSONValueObjectInsert(object, key, &value, false);
}
@@ -627,10 +630,8 @@ virJSONValueObjectInsertString(virJSONValuePtr object,
virJSONValuePtr jvalue = virJSONValueNewString(value);
if (!jvalue)
return -1;
- if (virJSONValueObjectInsert(object, key, jvalue, prepend) < 0) {
- virJSONValueFree(jvalue);
+ if (virJSONValueObjectInsert(object, key, &jvalue, prepend) < 0)
return -1;
- }
return 0;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 12/25] virJSONValueCopy: Don't use virJSONValue(Object|Array)Append
We know the exact number of keys or array members for the copied objects
so we can pre-allocate the arrays rather than inserting into them in a
loop incurring realloc copy penalty.
Also virJSONValueCopy now can't fail since all of the functions
allocating the different cases use just g_new/g_strdup internally so we
can remove the NULL checks from the recursive calls.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 80ebcb587c..d794eed17f 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1530,27 +1530,23 @@ virJSONValueCopy(const virJSONValue *in)
switch ((virJSONType) in->type) {
case VIR_JSON_TYPE_OBJECT:
out = virJSONValueNewObject();
+
+ out->data.object.pairs = g_new0(virJSONObjectPair,
in->data.object.npairs);
+ out->data.object.npairs = in->data.object.npairs;
+
for (i = 0; i < in->data.object.npairs; i++) {
- virJSONValuePtr val = NULL;
- if (!(val = virJSONValueCopy(in->data.object.pairs[i].value)))
- goto error;
- if (virJSONValueObjectAppend(out, in->data.object.pairs[i].key,
- val) < 0) {
- virJSONValueFree(val);
- goto error;
- }
+ out->data.object.pairs[i].key =
g_strdup(in->data.object.pairs[i].key);
+ out->data.object.pairs[i].value =
virJSONValueCopy(in->data.object.pairs[i].value);
}
break;
case VIR_JSON_TYPE_ARRAY:
out = virJSONValueNewArray();
+
+ out->data.array.values = g_new0(virJSONValuePtr, in->data.array.nvalues);
+ out->data.array.nvalues = in->data.array.nvalues;
+
for (i = 0; i < in->data.array.nvalues; i++) {
- virJSONValuePtr val = NULL;
- if (!(val = virJSONValueCopy(in->data.array.values[i])))
- goto error;
- if (virJSONValueArrayAppend(out, val) < 0) {
- virJSONValueFree(val);
- goto error;
- }
+ out->data.array.values[i] =
virJSONValueCopy(in->data.array.values[i]);
}
break;
@@ -1570,10 +1566,6 @@ virJSONValueCopy(const virJSONValue *in)
}
return out;
-
- error:
- virJSONValueFree(out);
- return NULL;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 13/25] virJSONValue(Array|Object)Append*: Simplify handling of appended object
Use g_autofree for the pointer of the added object and remove the NULL
checks for values returned by virJSONValueNew* (except
virJSONValueNewNumberDouble) since they can't fail nowadays.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 88 ++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 46 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index d794eed17f..adf1cfbcbc 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -676,13 +676,12 @@ virJSONValueObjectAppendNumberInt(virJSONValuePtr object,
const char *key,
int number)
{
- virJSONValuePtr jvalue = virJSONValueNewNumberInt(number);
- if (!jvalue)
- return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberInt(number);
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -692,13 +691,12 @@ virJSONValueObjectAppendNumberUint(virJSONValuePtr object,
const char *key,
unsigned int number)
{
- virJSONValuePtr jvalue = virJSONValueNewNumberUint(number);
- if (!jvalue)
- return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberUint(number);
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -708,13 +706,12 @@ virJSONValueObjectAppendNumberLong(virJSONValuePtr object,
const char *key,
long long number)
{
- virJSONValuePtr jvalue = virJSONValueNewNumberLong(number);
- if (!jvalue)
- return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberLong(number);
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -724,13 +721,12 @@ virJSONValueObjectAppendNumberUlong(virJSONValuePtr object,
const char *key,
unsigned long long number)
{
- virJSONValuePtr jvalue = virJSONValueNewNumberUlong(number);
- if (!jvalue)
- return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberUlong(number);
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -740,13 +736,16 @@ virJSONValueObjectAppendNumberDouble(virJSONValuePtr object,
const char *key,
double number)
{
- virJSONValuePtr jvalue = virJSONValueNewNumberDouble(number);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberDouble(number);
+
+ /* virJSONValueNewNumberDouble may return NULL if locale setting fails */
if (!jvalue)
return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -756,13 +755,12 @@ virJSONValueObjectAppendBoolean(virJSONValuePtr object,
const char *key,
int boolean_)
{
- virJSONValuePtr jvalue = virJSONValueNewBoolean(boolean_);
- if (!jvalue)
- return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewBoolean(boolean_);
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -771,13 +769,12 @@ int
virJSONValueObjectAppendNull(virJSONValuePtr object,
const char *key)
{
- virJSONValuePtr jvalue = virJSONValueNewNull();
- if (!jvalue)
- return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewNull();
+
+ if (virJSONValueObjectAppend(object, key, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
@@ -806,13 +803,12 @@ int
virJSONValueArrayAppendString(virJSONValuePtr object,
const char *value)
{
- virJSONValuePtr jvalue = virJSONValueNewString(value);
- if (!jvalue)
- return -1;
- if (virJSONValueArrayAppend(object, jvalue) < 0) {
- virJSONValueFree(jvalue);
+ g_autoptr(virJSONValue) jvalue = virJSONValueNewString(value);
+
+ if (virJSONValueArrayAppend(object, jvalue) < 0)
return -1;
- }
+ jvalue = NULL;
+
return 0;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 14/25] virJSONValueNewArrayFromBitmap: Refactor cleanup
Use g_autoptr for the JSON value objects and remove the cleanup label
and inline freeing of objects.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index adf1cfbcbc..e4d71d3e09 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1241,29 +1241,21 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val,
virJSONValuePtr
virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap)
{
- virJSONValuePtr ret;
+ g_autoptr(virJSONValue) ret = virJSONValueNewArray();
ssize_t pos = -1;
- ret = virJSONValueNewArray();
-
if (!bitmap)
- return ret;
+ return g_steal_pointer(&ret);
while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) {
- virJSONValuePtr newelem;
+ g_autoptr(virJSONValue) newelem = virJSONValueNewNumberLong(pos);
- if (!(newelem = virJSONValueNewNumberLong(pos)) ||
- virJSONValueArrayAppend(ret, newelem) < 0) {
- virJSONValueFree(newelem);
- goto error;
- }
+ if (virJSONValueArrayAppend(ret, newelem) < 0)
+ return NULL;
+ newelem = NULL;
}
- return ret;
-
- error:
- virJSONValueFree(ret);
- return NULL;
+ return g_steal_pointer(&ret);
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 15/25] virJSONValueObjectAddVArgs: Use autofree for the temporary bitmap
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index e4d71d3e09..7b52525797 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -305,7 +305,7 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
case 'M':
case 'm': {
virBitmapPtr map = va_arg(args, virBitmapPtr);
- virJSONValuePtr jsonMap;
+ g_autoptr(virJSONValue) jsonMap = NULL;
if (!map) {
if (type == 'M')
@@ -321,7 +321,8 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
return -1;
if ((rc = virJSONValueObjectAppend(obj, key, jsonMap)) < 0)
- virJSONValueFree(jsonMap);
+ return -1;
+ jsonMap = NULL;
} break;
default:
--
2.29.2
6:55 p.m.
New subject: [PATCH 16/25] virJSONValueObjectAppend: Clear pointer when taking ownership of passed value
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/locking/lock_daemon.c | 9 +++----
src/logging/log_daemon.c | 6 ++---
src/logging/log_handler.c | 3 +--
src/node_device/node_device_driver.c | 2 +-
src/qemu/qemu_block.c | 3 +--
src/qemu/qemu_firmware.c | 23 +++++-----------
src/qemu/qemu_monitor_json.c | 22 ++++++----------
src/rpc/virnetdaemon.c | 6 ++---
src/rpc/virnetserver.c | 6 ++---
src/rpc/virnetserverclient.c | 6 ++---
src/rpc/virnetserverservice.c | 3 +--
src/util/virjson.c | 39 ++++++++++++----------------
src/util/virjson.h | 4 ++-
src/util/virlockspace.c | 6 ++---
src/util/virmacmap.c | 3 +--
15 files changed, 52 insertions(+), 89 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 5913c0cb9c..26905a462b 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -711,16 +711,14 @@ virLockDaemonPreExecRestart(const char *state_file,
if (!(daemon = virNetDaemonPreExecRestart(dmn)))
return -1;
- if (virJSONValueObjectAppend(object, "daemon", daemon) < 0)
+ if (virJSONValueObjectAppend(object, "daemon", &daemon) < 0)
return -1;
- daemon = NULL;
if (!(defaultLockspace =
virLockSpacePreExecRestart(lockDaemon->defaultLockspace)))
return -1;
- if (virJSONValueObjectAppend(object, "defaultLockspace", defaultLockspace)
< 0)
+ if (virJSONValueObjectAppend(object, "defaultLockspace",
&defaultLockspace) < 0)
return -1;
- defaultLockspace = NULL;
tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL, false);
while (tmp && tmp->key) {
@@ -737,9 +735,8 @@ virLockDaemonPreExecRestart(const char *state_file,
tmp++;
}
- if (virJSONValueObjectAppend(object, "lockspaces", lockspaces) < 0)
+ if (virJSONValueObjectAppend(object, "lockspaces", &lockspaces) <
0)
return -1;
- lockspaces = NULL;
if (!(magic = virLockDaemonGetExecRestartMagic()))
return -1;
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index ceffa6a368..cc0b96ea05 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -516,9 +516,8 @@ virLogDaemonPreExecRestart(const char *state_file,
if (!(daemon = virNetDaemonPreExecRestart(dmn)))
return -1;
- if (virJSONValueObjectAppend(object, "daemon", daemon) < 0)
+ if (virJSONValueObjectAppend(object, "daemon", &daemon) < 0)
return -1;
- daemon = NULL;
if (!(magic = virLogDaemonGetExecRestartMagic()))
return -1;
@@ -529,9 +528,8 @@ virLogDaemonPreExecRestart(const char *state_file,
if (!(handler = virLogHandlerPreExecRestart(logDaemon->handler)))
return -1;
- if (virJSONValueObjectAppend(object, "handler", handler) < 0)
+ if (virJSONValueObjectAppend(object, "handler", &handler) < 0)
return -1;
- handler = NULL;
if (!(state = virJSONValueToString(object, true)))
return -1;
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index cacf9584cd..d649c4d132 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -647,9 +647,8 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler)
file = NULL;
}
- if (virJSONValueObjectAppend(ret, "files", files) < 0)
+ if (virJSONValueObjectAppend(ret, "files", &files) < 0)
return NULL;
- files = NULL;
return g_steal_pointer(&ret);
}
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 51b20848ce..d946e64ad6 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -610,7 +610,7 @@ nodeDeviceDefToMdevctlConfig(virNodeDeviceDefPtr def, char **buf)
return -1;
}
- if (virJSONValueObjectAppend(json, "attrs",
g_steal_pointer(&attributes)) < 0)
+ if (virJSONValueObjectAppend(json, "attrs", &attributes) < 0)
return -1;
}
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 3d88e701b2..1df51ba97b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1097,9 +1097,8 @@ qemuBlockStorageSourceGetBlockdevGetCacheProps(virStorageSourcePtr
src,
NULL) < 0)
return -1;
- if (virJSONValueObjectAppend(props, "cache", cacheobj) < 0)
+ if (virJSONValueObjectAppend(props, "cache", &cacheobj) < 0)
return -1;
- cacheobj = NULL;
return 0;
}
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index c571fdecf7..5e8fdd0ff1 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -662,10 +662,9 @@ qemuFirmwareInterfaceFormat(virJSONValuePtr doc,
if (virJSONValueObjectAppend(doc,
"interface-types",
- interfacesJSON) < 0)
+ &interfacesJSON) < 0)
return -1;
- interfacesJSON = NULL;
return 0;
}
@@ -706,18 +705,15 @@ qemuFirmwareMappingFlashFormat(virJSONValuePtr mapping,
if (virJSONValueObjectAppend(mapping,
"executable",
- executable) < 0)
+ &executable) < 0)
return -1;
- executable = NULL;
if (virJSONValueObjectAppend(mapping,
"nvram-template",
- nvram_template) < 0)
+ &nvram_template) < 0)
return -1;
- nvram_template = NULL;
-
return 0;
}
@@ -778,10 +774,9 @@ qemuFirmwareMappingFormat(virJSONValuePtr doc,
break;
}
- if (virJSONValueObjectAppend(doc, "mapping", mapping) < 0)
+ if (virJSONValueObjectAppend(doc, "mapping", &mapping) < 0)
return -1;
- mapping = NULL;
return 0;
}
@@ -814,21 +809,18 @@ qemuFirmwareTargetFormat(virJSONValuePtr doc,
return -1;
}
- if (virJSONValueObjectAppend(target, "machines", machines) < 0)
+ if (virJSONValueObjectAppend(target, "machines", &machines) <
0)
return -1;
- machines = NULL;
-
if (virJSONValueArrayAppend(targetsJSON, target) < 0)
return -1;
target = NULL;
}
- if (virJSONValueObjectAppend(doc, "targets", targetsJSON) < 0)
+ if (virJSONValueObjectAppend(doc, "targets", &targetsJSON) < 0)
return -1;
- targetsJSON = NULL;
return 0;
}
@@ -850,10 +842,9 @@ qemuFirmwareFeatureFormat(virJSONValuePtr doc,
if (virJSONValueObjectAppend(doc,
"features",
- featuresJSON) < 0)
+ &featuresJSON) < 0)
return -1;
- featuresJSON = NULL;
return 0;
}
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f8c78d9093..924e03b4da 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4554,9 +4554,8 @@ qemuMonitorJSONAddDeviceArgs(qemuMonitorPtr mon,
if (!(cmd = qemuMonitorJSONMakeCommand("device_add", NULL)))
goto cleanup;
- if (virJSONValueObjectAppend(cmd, "arguments", args) < 0)
+ if (virJSONValueObjectAppend(cmd, "arguments", &args) < 0)
goto cleanup;
- args = NULL; /* obj owns reference to args now */
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
@@ -5572,9 +5571,8 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
NULL) < 0)
goto cleanup;
- if (virJSONValueObjectAppend(cmd, "arguments", args) < 0)
+ if (virJSONValueObjectAppend(cmd, "arguments", &args) < 0)
goto cleanup;
- args = NULL; /* obj owns reference to args now */
if (qemuMonitorJSONCommand(mon, cmd, &result) < 0)
goto cleanup;
@@ -5989,7 +5987,7 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu,
goto error;
}
- if (virJSONValueObjectAppend(model, "props", props) < 0)
+ if (virJSONValueObjectAppend(model, "props", &props) < 0)
goto error;
}
@@ -7470,9 +7468,8 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.tcp.host,
chr->data.tcp.service);
if (!addr ||
- virJSONValueObjectAppend(data, "addr", addr) < 0)
+ virJSONValueObjectAppend(data, "addr", &addr) < 0)
goto cleanup;
- addr = NULL;
telnet = chr->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
@@ -7503,7 +7500,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
addr = qemuMonitorJSONBuildInetSocketAddress(host,
chr->data.udp.connectService);
if (!addr ||
- virJSONValueObjectAppend(data, "remote", addr) < 0)
+ virJSONValueObjectAppend(data, "remote", &addr) < 0)
goto cleanup;
host = chr->data.udp.bindHost;
@@ -7515,10 +7512,9 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
port = "";
addr = qemuMonitorJSONBuildInetSocketAddress(host, port);
if (!addr ||
- virJSONValueObjectAppend(data, "local", addr) < 0)
+ virJSONValueObjectAppend(data, "local", &addr) < 0)
goto cleanup;
}
- addr = NULL;
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -7526,9 +7522,8 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
addr = qemuMonitorJSONBuildUnixSocketAddress(chr->data.nix.path);
if (!addr ||
- virJSONValueObjectAppend(data, "addr", addr) < 0)
+ virJSONValueObjectAppend(data, "addr", &addr) < 0)
goto cleanup;
- addr = NULL;
if (chr->data.nix.listen &&
virJSONValueObjectAppendBoolean(data, "wait", false) < 0)
@@ -7574,9 +7569,8 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
goto cleanup;
if (virJSONValueObjectAppendString(backend, "type", backend_type) < 0
||
- virJSONValueObjectAppend(backend, "data", data) < 0)
+ virJSONValueObjectAppend(backend, "data", &data) < 0)
goto cleanup;
- data = NULL;
if (!(ret = qemuMonitorJSONMakeCommand("chardev-add",
"s:id", chrID,
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 6132615093..327540c4b4 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -404,16 +404,14 @@ virNetDaemonPreExecRestart(virNetDaemonPtr dmn)
if (!srvJSON)
goto error;
- if (virJSONValueObjectAppend(srvObj, srvArray[i].key, srvJSON) < 0)
+ if (virJSONValueObjectAppend(srvObj, srvArray[i].key, &srvJSON) < 0)
goto error;
- srvJSON = NULL;
}
virObjectUnlock(dmn);
- if (virJSONValueObjectAppend(object, "servers", srvObj) < 0)
+ if (virJSONValueObjectAppend(object, "servers", &srvObj) < 0)
return NULL;
- srvObj = NULL;
return g_steal_pointer(&object);
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index ee402e8ea0..c2650ade09 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -590,9 +590,8 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
child = NULL;
}
- if (virJSONValueObjectAppend(object, "services", services) < 0)
+ if (virJSONValueObjectAppend(object, "services", &services) < 0)
goto error;
- services = NULL;
for (i = 0; i < srv->nclients; i++) {
g_autoptr(virJSONValue) child = NULL;
@@ -604,9 +603,8 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
child = NULL;
}
- if (virJSONValueObjectAppend(object, "clients", clients) < 0)
+ if (virJSONValueObjectAppend(object, "clients", &clients) < 0)
goto error;
- clients = NULL;
virObjectUnlock(srv);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 0789ad9154..2bb3443287 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -610,16 +610,14 @@ virJSONValuePtr
virNetServerClientPreExecRestart(virNetServerClientPtr client)
if (!(sock = virNetSocketPreExecRestart(client->sock)))
goto error;
- if (virJSONValueObjectAppend(object, "sock", sock) < 0)
+ if (virJSONValueObjectAppend(object, "sock", &sock) < 0)
goto error;
- sock = NULL;
if (!(priv = client->privateDataPreExecRestart(client, client->privateData)))
goto error;
- if (virJSONValueObjectAppend(object, "privateData", priv) < 0)
+ if (virJSONValueObjectAppend(object, "privateData", &priv) < 0)
goto error;
- priv = NULL;
virObjectUnlock(client);
return g_steal_pointer(&object);
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index a72277226a..fece6305e8 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -357,9 +357,8 @@ virJSONValuePtr
virNetServerServicePreExecRestart(virNetServerServicePtr svc)
child = NULL;
}
- if (virJSONValueObjectAppend(object, "socks", socks) < 0)
+ if (virJSONValueObjectAppend(object, "socks", &socks) < 0)
return NULL;
- socks = NULL;
return g_steal_pointer(&object);
}
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 7b52525797..5b3aa49a7f 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -298,7 +298,7 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
return -1;
}
- if ((rc = virJSONValueObjectAppend(obj, key, *val)) == 0)
+ if ((rc = virJSONValueObjectAppend(obj, key, val)) == 0)
*val = NULL;
} break;
@@ -320,9 +320,8 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
if (!(jsonMap = virJSONValueNewArrayFromBitmap(map)))
return -1;
- if ((rc = virJSONValueObjectAppend(obj, key, jsonMap)) < 0)
+ if ((rc = virJSONValueObjectAppend(obj, key, &jsonMap)) < 0)
return -1;
- jsonMap = NULL;
} break;
default:
@@ -616,9 +615,9 @@ virJSONValueObjectInsert(virJSONValuePtr object,
int
virJSONValueObjectAppend(virJSONValuePtr object,
const char *key,
- virJSONValuePtr value)
+ virJSONValuePtr *value)
{
- return virJSONValueObjectInsert(object, key, &value, false);
+ return virJSONValueObjectInsert(object, key, value, false);
}
@@ -679,9 +678,8 @@ virJSONValueObjectAppendNumberInt(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberInt(number);
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -694,9 +692,8 @@ virJSONValueObjectAppendNumberUint(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberUint(number);
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -709,9 +706,8 @@ virJSONValueObjectAppendNumberLong(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberLong(number);
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -724,9 +720,8 @@ virJSONValueObjectAppendNumberUlong(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewNumberUlong(number);
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -743,9 +738,8 @@ virJSONValueObjectAppendNumberDouble(virJSONValuePtr object,
if (!jvalue)
return -1;
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -758,9 +752,8 @@ virJSONValueObjectAppendBoolean(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewBoolean(boolean_);
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -772,9 +765,8 @@ virJSONValueObjectAppendNull(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewNull();
- if (virJSONValueObjectAppend(object, key, jvalue) < 0)
+ if (virJSONValueObjectAppend(object, key, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -1583,7 +1575,7 @@ virJSONParserInsertValue(virJSONParserPtr parser,
if (virJSONValueObjectAppend(state->value,
state->key,
- value) < 0)
+ &value) < 0)
return -1;
VIR_FREE(state->key);
@@ -2093,7 +2085,7 @@ virJSONValueObjectDeflattenWorker(const char *key,
return -1;
}
- if (virJSONValueObjectAppend(retobj, key, newval) < 0)
+ if (virJSONValueObjectAppend(retobj, key, &newval) < 0)
return -1;
newval = NULL;
@@ -2111,9 +2103,10 @@ virJSONValueObjectDeflattenWorker(const char *key,
}
if (!(existobj = virJSONValueObjectGet(retobj, tokens[0]))) {
- existobj = virJSONValueNewObject();
+ virJSONValuePtr newobj = virJSONValueNewObject();
+ existobj = newobj;
- if (virJSONValueObjectAppend(retobj, tokens[0], existobj) < 0)
+ if (virJSONValueObjectAppend(retobj, tokens[0], &newobj) < 0)
return -1;
} else {
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 4dc7ed1a2d..ca99ae3b70 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -69,7 +69,9 @@ virJSONValuePtr virJSONValueNewArray(void);
virJSONValuePtr virJSONValueNewObject(void);
virJSONValuePtr virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap);
-int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONValuePtr
value);
+int virJSONValueObjectAppend(virJSONValuePtr object,
+ const char *key,
+ virJSONValuePtr *value);
int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value);
int virJSONValueArrayConcat(virJSONValuePtr a,
virJSONValuePtr c);
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 1b6b51b649..9ba6e83251 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -460,9 +460,8 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
owner = NULL;
}
- if (virJSONValueObjectAppend(child, "owners", owners) < 0)
+ if (virJSONValueObjectAppend(child, "owners", &owners) < 0)
goto error;
- owners = NULL;
if (virJSONValueArrayAppend(resources, child) < 0)
goto error;
@@ -471,9 +470,8 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
tmp++;
}
- if (virJSONValueObjectAppend(object, "resources", resources) < 0)
+ if (virJSONValueObjectAppend(object, "resources", &resources) < 0)
goto error;
- resources = NULL;
virMutexUnlock(&lockspace->lock);
return object;
diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
index 0d458d7b7b..521ab89b5b 100644
--- a/src/util/virmacmap.c
+++ b/src/util/virmacmap.c
@@ -224,9 +224,8 @@ virMACMapHashDumper(void *payload,
}
if (virJSONValueObjectAppendString(obj, "domain", name) < 0 ||
- virJSONValueObjectAppend(obj, "macs", arr) < 0)
+ virJSONValueObjectAppend(obj, "macs", &arr) < 0)
return -1;
- arr = NULL;
if (virJSONValueArrayAppend(data, obj) < 0)
return -1;
--
2.29.2
6:55 p.m.
New subject: [PATCH 17/25] qemuAgentMakeStringsArray: Refactor cleanup
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_agent.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 4712aeb529..d6816ef9de 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1175,23 +1175,17 @@ static virJSONValuePtr
qemuAgentMakeStringsArray(const char **strings, unsigned int len)
{
size_t i;
- virJSONValuePtr ret = virJSONValueNewArray(), str;
+ g_autoptr(virJSONValue) ret = virJSONValueNewArray();
for (i = 0; i < len; i++) {
- str = virJSONValueNewString(strings[i]);
- if (!str)
- goto error;
+ g_autoptr(virJSONValue) str = virJSONValueNewString(strings[i]);
- if (virJSONValueArrayAppend(ret, str) < 0) {
- virJSONValueFree(str);
- goto error;
- }
+ if (virJSONValueArrayAppend(ret, str) < 0)
+ return NULL;
+ str = NULL;
}
- return ret;
- error:
- virJSONValueFree(ret);
- return NULL;
+ return g_steal_pointer(&ret);
}
void qemuAgentNotifyEvent(qemuAgentPtr agent,
--
2.29.2
6:55 p.m.
New subject: [PATCH 18/25] virMACMapHashDumper: Refactor array addition
Use automatic memory freeing and don't check return value of
virJSONValueNewString as it can't fail.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virmacmap.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
index 521ab89b5b..12df325933 100644
--- a/src/util/virmacmap.c
+++ b/src/util/virmacmap.c
@@ -214,13 +214,11 @@ virMACMapHashDumper(void *payload,
GSList *next;
for (next = macs; next; next = next->next) {
- virJSONValuePtr m = virJSONValueNewString((const char *) next->data);
+ g_autoptr(virJSONValue) m = virJSONValueNewString((const char *) next->data);
- if (!m ||
- virJSONValueArrayAppend(arr, m) < 0) {
- virJSONValueFree(m);
+ if (virJSONValueArrayAppend(arr, m) < 0)
return -1;
- }
+ m = NULL;
}
if (virJSONValueObjectAppendString(obj, "domain", name) < 0 ||
--
2.29.2
6:55 p.m.
New subject: [PATCH 19/25] testQEMUSchemaValidateObjectMergeVariantMember: Fix theoretical leak
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/testutilsqemuschema.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 101687e657..4bb303a427 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -163,13 +163,14 @@ testQEMUSchemaValidateObjectMergeVariantMember(size_t pos
G_GNUC_UNUSED,
void *opaque)
{
virJSONValuePtr array = opaque;
- virJSONValuePtr copy;
+ g_autoptr(virJSONValue) copy = NULL;
if (!(copy = virJSONValueCopy(item)))
return -1;
if (virJSONValueArrayAppend(array, copy) < 0)
return -1;
+ copy = NULL;
return 1;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 20/25] virJSONValueArrayAppend: Clear pointer when taking ownership of passed value
The parent array takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/locking/lock_daemon.c | 3 +--
src/logging/log_handler.c | 3 +--
src/network/leaseshelper.c | 3 +--
src/node_device/node_device_driver.c | 2 +-
src/qemu/qemu_agent.c | 7 ++-----
src/qemu/qemu_block.c | 16 ++++------------
src/qemu/qemu_command.c | 3 +--
src/qemu/qemu_firmware.c | 4 +---
src/qemu/qemu_migration_params.c | 4 +---
src/qemu/qemu_monitor_json.c | 11 +++--------
src/rpc/virnetserver.c | 6 ++----
src/rpc/virnetserverservice.c | 3 +--
src/util/virjson.c | 12 +++++-------
src/util/virjson.h | 3 ++-
src/util/virlease.c | 2 +-
src/util/virlockspace.c | 6 ++----
src/util/virmacmap.c | 6 ++----
tests/testutilsqemuschema.c | 3 +--
18 files changed, 32 insertions(+), 65 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 26905a462b..d68c61b099 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -728,9 +728,8 @@ virLockDaemonPreExecRestart(const char *state_file,
if (!(child = virLockSpacePreExecRestart(lockspace)))
return -1;
- if (virJSONValueArrayAppend(lockspaces, child) < 0)
+ if (virJSONValueArrayAppend(lockspaces, &child) < 0)
return -1;
- child = NULL;
tmp++;
}
diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
index d649c4d132..9e027c7579 100644
--- a/src/logging/log_handler.c
+++ b/src/logging/log_handler.c
@@ -642,9 +642,8 @@ virLogHandlerPreExecRestart(virLogHandlerPtr handler)
return NULL;
}
- if (virJSONValueArrayAppend(files, file) < 0)
+ if (virJSONValueArrayAppend(files, &file) < 0)
return NULL;
- file = NULL;
}
if (virJSONValueObjectAppend(ret, "files", &files) < 0)
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index c20e63efa9..ca2c640672 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -226,12 +226,11 @@ main(int argc, char **argv)
case VIR_LEASE_ACTION_OLD:
case VIR_LEASE_ACTION_ADD:
- if (lease_new && virJSONValueArrayAppend(leases_array_new, lease_new)
< 0) {
+ if (lease_new && virJSONValueArrayAppend(leases_array_new,
&lease_new) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to create json"));
goto cleanup;
}
- lease_new = NULL;
G_GNUC_FALLTHROUGH;
case VIR_LEASE_ACTION_DEL:
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index d946e64ad6..543e5bb90a 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -606,7 +606,7 @@ nodeDeviceDefToMdevctlConfig(virNodeDeviceDefPtr def, char **buf)
if (virJSONValueObjectAppendString(jsonattr, attr->name, attr->value)
< 0)
return -1;
- if (virJSONValueArrayAppend(attributes, g_steal_pointer(&jsonattr)) <
0)
+ if (virJSONValueArrayAppend(attributes, &jsonattr) < 0)
return -1;
}
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index d6816ef9de..9aec0fdb4b 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1180,9 +1180,8 @@ qemuAgentMakeStringsArray(const char **strings, unsigned int len)
for (i = 0; i < len; i++) {
g_autoptr(virJSONValue) str = virJSONValueNewString(strings[i]);
- if (virJSONValueArrayAppend(ret, str) < 0)
+ if (virJSONValueArrayAppend(ret, &str) < 0)
return NULL;
- str = NULL;
}
return g_steal_pointer(&ret);
@@ -1514,10 +1513,8 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr agent,
if (virJSONValueObjectAppendBoolean(cpu, "online", in->online) <
0)
goto cleanup;
- if (virJSONValueArrayAppend(cpus, cpu) < 0)
+ if (virJSONValueArrayAppend(cpus, &cpu) < 0)
goto cleanup;
-
- cpu = NULL;
}
if (*nmodified == 0) {
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 1df51ba97b..94cbb1d12e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -534,10 +534,8 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr
src,
if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(host, legacy)))
return NULL;
- if (virJSONValueArrayAppend(servers, server) < 0)
+ if (virJSONValueArrayAppend(servers, &server) < 0)
return NULL;
-
- server = NULL;
}
return g_steal_pointer(&servers);
@@ -622,10 +620,8 @@
qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src)
if (!(server = qemuBlockStorageSourceBuildJSONInetSocketAddress(host)))
return NULL;
- if (virJSONValueArrayAppend(servers, server) < 0)
+ if (virJSONValueArrayAppend(servers, &server) < 0)
return NULL;
-
- server = NULL;
}
return g_steal_pointer(&servers);
@@ -912,16 +908,12 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src,
authmodes = virJSONValueNewArray();
if (!(mode = virJSONValueNewString("cephx")) ||
- virJSONValueArrayAppend(authmodes, mode) < 0)
+ virJSONValueArrayAppend(authmodes, &mode) < 0)
return NULL;
- mode = NULL;
-
if (!(mode = virJSONValueNewString("none")) ||
- virJSONValueArrayAppend(authmodes, mode) < 0)
+ virJSONValueArrayAppend(authmodes, &mode) < 0)
return NULL;
-
- mode = NULL;
}
if (virJSONValueObjectCreate(&ret,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d27d5eb55b..3037d1d5a5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10200,9 +10200,8 @@ qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr)
chr->info.alias) < 0)
return NULL;
- if (virJSONValueArrayAppend(guestfwdarr, guestfwdstrobj) < 0)
+ if (virJSONValueArrayAppend(guestfwdarr, &guestfwdstrobj) < 0)
return NULL;
- guestfwdstrobj = NULL;
if (virJSONValueObjectCreate(&ret,
"s:type", "user",
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 5e8fdd0ff1..d3198e2d45 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -812,10 +812,8 @@ qemuFirmwareTargetFormat(virJSONValuePtr doc,
if (virJSONValueObjectAppend(target, "machines", &machines) <
0)
return -1;
- if (virJSONValueArrayAppend(targetsJSON, target) < 0)
+ if (virJSONValueArrayAppend(targetsJSON, &target) < 0)
return -1;
-
- target = NULL;
}
if (virJSONValueObjectAppend(doc, "targets", &targetsJSON) < 0)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 510dad783a..302a9933e1 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -770,10 +770,8 @@ qemuMigrationCapsToJSON(virBitmapPtr caps,
NULL) < 0)
return NULL;
- if (virJSONValueArrayAppend(json, cap) < 0)
+ if (virJSONValueArrayAppend(json, &cap) < 0)
return NULL;
-
- cap = NULL;
}
return g_steal_pointer(&json);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 924e03b4da..94365c775b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -525,10 +525,9 @@ qemuMonitorJSONTransactionAdd(virJSONValuePtr actions,
"A:data", &data, NULL) < 0)
goto cleanup;
- if (virJSONValueArrayAppend(actions, entry) < 0)
+ if (virJSONValueArrayAppend(actions, &entry) < 0)
goto cleanup;
- entry = NULL;
ret = 0;
cleanup:
@@ -4976,11 +4975,8 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
if (virJSONValueObjectAppendNumberInt(key, "data", keycodes[i]) <
0)
goto cleanup;
- if (virJSONValueArrayAppend(keys, key) < 0)
+ if (virJSONValueArrayAppend(keys, &key) < 0)
goto cleanup;
-
- key = NULL;
-
}
cmd = qemuMonitorJSONMakeCommand("send-key",
@@ -9289,10 +9285,9 @@
qemuMonitorJSONTransactionBitmapMergeSourceAddBitmap(virJSONValuePtr sources,
NULL) < 0)
return -1;
- if (virJSONValueArrayAppend(sources, sourceobj) < 0)
+ if (virJSONValueArrayAppend(sources, &sourceobj) < 0)
return -1;
- sourceobj = NULL;
return 0;
}
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index c2650ade09..30faa0190b 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -585,9 +585,8 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
if (!(child = virNetServerServicePreExecRestart(srv->services[i])))
goto error;
- if (virJSONValueArrayAppend(services, child) < 0)
+ if (virJSONValueArrayAppend(services, &child) < 0)
goto error;
- child = NULL;
}
if (virJSONValueObjectAppend(object, "services", &services) < 0)
@@ -598,9 +597,8 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
if (!(child = virNetServerClientPreExecRestart(srv->clients[i])))
goto error;
- if (virJSONValueArrayAppend(clients, child) < 0)
+ if (virJSONValueArrayAppend(clients, &child) < 0)
goto error;
- child = NULL;
}
if (virJSONValueObjectAppend(object, "clients", &clients) < 0)
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index fece6305e8..ce2dd4f28b 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -352,9 +352,8 @@ virJSONValuePtr
virNetServerServicePreExecRestart(virNetServerServicePtr svc)
if (!(child = virNetSocketPreExecRestart(svc->socks[i])))
return NULL;
- if (virJSONValueArrayAppend(socks, child) < 0)
+ if (virJSONValueArrayAppend(socks, &child) < 0)
return NULL;
- child = NULL;
}
if (virJSONValueObjectAppend(object, "socks", &socks) < 0)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 5b3aa49a7f..65215f03c5 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -774,7 +774,7 @@ virJSONValueObjectAppendNull(virJSONValuePtr object,
int
virJSONValueArrayAppend(virJSONValuePtr array,
- virJSONValuePtr value)
+ virJSONValuePtr *value)
{
if (array->type != VIR_JSON_TYPE_ARRAY) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("expecting JSON
array"));
@@ -785,7 +785,7 @@ virJSONValueArrayAppend(virJSONValuePtr array,
array->data.array.nvalues + 1) < 0)
return -1;
- array->data.array.values[array->data.array.nvalues] = value;
+ array->data.array.values[array->data.array.nvalues] = g_steal_pointer(value);
array->data.array.nvalues++;
return 0;
@@ -798,9 +798,8 @@ virJSONValueArrayAppendString(virJSONValuePtr object,
{
g_autoptr(virJSONValue) jvalue = virJSONValueNewString(value);
- if (virJSONValueArrayAppend(object, jvalue) < 0)
+ if (virJSONValueArrayAppend(object, &jvalue) < 0)
return -1;
- jvalue = NULL;
return 0;
}
@@ -1243,9 +1242,8 @@ virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap)
while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) {
g_autoptr(virJSONValue) newelem = virJSONValueNewNumberLong(pos);
- if (virJSONValueArrayAppend(ret, newelem) < 0)
+ if (virJSONValueArrayAppend(ret, &newelem) < 0)
return NULL;
- newelem = NULL;
}
return g_steal_pointer(&ret);
@@ -1588,7 +1586,7 @@ virJSONParserInsertValue(virJSONParserPtr parser,
}
if (virJSONValueArrayAppend(state->value,
- value) < 0)
+ &value) < 0)
return -1;
} break;
diff --git a/src/util/virjson.h b/src/util/virjson.h
index ca99ae3b70..c22bc1fb45 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -72,7 +72,8 @@ virJSONValuePtr virJSONValueNewArrayFromBitmap(virBitmapPtr bitmap);
int virJSONValueObjectAppend(virJSONValuePtr object,
const char *key,
virJSONValuePtr *value);
-int virJSONValueArrayAppend(virJSONValuePtr object, virJSONValuePtr value);
+int virJSONValueArrayAppend(virJSONValuePtr object,
+ virJSONValuePtr *value);
int virJSONValueArrayConcat(virJSONValuePtr a,
virJSONValuePtr c);
diff --git a/src/util/virlease.c b/src/util/virlease.c
index 3d68bb660c..59eaabd4d9 100644
--- a/src/util/virlease.c
+++ b/src/util/virlease.c
@@ -119,7 +119,7 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new,
}
/* Move old lease to new array */
- if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) {
+ if (virJSONValueArrayAppend(leases_array_new, &lease_tmp) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to create json"));
return -1;
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 9ba6e83251..684a3320ed 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -455,17 +455,15 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr
lockspace)
if (!owner)
goto error;
- if (virJSONValueArrayAppend(owners, owner) < 0)
+ if (virJSONValueArrayAppend(owners, &owner) < 0)
goto error;
- owner = NULL;
}
if (virJSONValueObjectAppend(child, "owners", &owners) < 0)
goto error;
- if (virJSONValueArrayAppend(resources, child) < 0)
+ if (virJSONValueArrayAppend(resources, &child) < 0)
goto error;
- child = NULL;
tmp++;
}
diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
index 12df325933..bb14513c30 100644
--- a/src/util/virmacmap.c
+++ b/src/util/virmacmap.c
@@ -216,18 +216,16 @@ virMACMapHashDumper(void *payload,
for (next = macs; next; next = next->next) {
g_autoptr(virJSONValue) m = virJSONValueNewString((const char *) next->data);
- if (virJSONValueArrayAppend(arr, m) < 0)
+ if (virJSONValueArrayAppend(arr, &m) < 0)
return -1;
- m = NULL;
}
if (virJSONValueObjectAppendString(obj, "domain", name) < 0 ||
virJSONValueObjectAppend(obj, "macs", &arr) < 0)
return -1;
- if (virJSONValueArrayAppend(data, obj) < 0)
+ if (virJSONValueArrayAppend(data, &obj) < 0)
return -1;
- obj = NULL;
return 0;
}
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 4bb303a427..a842a219e2 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -168,9 +168,8 @@ testQEMUSchemaValidateObjectMergeVariantMember(size_t pos
G_GNUC_UNUSED,
if (!(copy = virJSONValueCopy(item)))
return -1;
- if (virJSONValueArrayAppend(array, copy) < 0)
+ if (virJSONValueArrayAppend(array, ©) < 0)
return -1;
- copy = NULL;
return 1;
}
--
2.29.2
10:11 a.m.
New subject: [PATCH 20/25] virJSONValueArrayAppend: Clear pointer when taking ownership of passed value
On 2/12/21 6:55 PM, Peter Krempa wrote:
The parent array takes ownership of the inserted value once all
checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/locking/lock_daemon.c | 3 +--
src/logging/log_handler.c | 3 +--
src/network/leaseshelper.c | 3 +--
src/node_device/node_device_driver.c | 2 +-
src/qemu/qemu_agent.c | 7 ++-----
src/qemu/qemu_block.c | 16 ++++------------
src/qemu/qemu_command.c | 3 +--
src/qemu/qemu_firmware.c | 4 +---
src/qemu/qemu_migration_params.c | 4 +---
src/qemu/qemu_monitor_json.c | 11 +++--------
src/rpc/virnetserver.c | 6 ++----
src/rpc/virnetserverservice.c | 3 +--
src/util/virjson.c | 12 +++++-------
src/util/virjson.h | 3 ++-
src/util/virlease.c | 2 +-
src/util/virlockspace.c | 6 ++----
src/util/virmacmap.c | 6 ++----
tests/testutilsqemuschema.c | 3 +--
18 files changed, 32 insertions(+), 65 deletions(-)
I think the following should be squashed in:
diff --git i/src/qemu/qemu_monitor_json.c w/src/qemu/qemu_monitor_json.c
index 924e03b4da..2e040a2bc1 100644
--- i/src/qemu/qemu_monitor_json.c
+++ w/src/qemu/qemu_monitor_json.c
@@ -7580,7 +7580,6 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
cleanup:
VIR_FREE(tlsalias);
- virJSONValueFree(addr);
virJSONValueFree(data);
virJSONValueFree(backend);
return ret;
diff --git i/src/util/virjson.c w/src/util/virjson.c
index 5b3aa49a7f..69b3c3c63f 100644
--- i/src/util/virjson.c
+++ w/src/util/virjson.c
@@ -298,8 +298,7 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
return -1;
}
- if ((rc = virJSONValueObjectAppend(obj, key, val)) == 0)
- *val = NULL;
+ rc = virJSONValueObjectAppend(obj, key, val);
} break;
case 'M':
@@ -2088,8 +2087,6 @@ virJSONValueObjectDeflattenWorker(const char *key,
if (virJSONValueObjectAppend(retobj, key, &newval) < 0)
return -1;
- newval = NULL;
-
return 0;
}
Michal
1:25 p.m.
New subject: [PATCH 20/25] virJSONValueArrayAppend: Clear pointer when taking ownership of passed value
On Mon, Feb 15, 2021 at 10:11:19 +0100, Michal Privoznik wrote:
On 2/12/21 6:55 PM, Peter Krempa wrote:
> The parent array takes ownership of the inserted value once all checks
> pass. Don't make the callers second-guess when that happens and modify
> the function to take a double pointer so that it can be cleared once the
> ownership is taken.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/locking/lock_daemon.c | 3 +--
> src/logging/log_handler.c | 3 +--
> src/network/leaseshelper.c | 3 +--
> src/node_device/node_device_driver.c | 2 +-
> src/qemu/qemu_agent.c | 7 ++-----
> src/qemu/qemu_block.c | 16 ++++------------
> src/qemu/qemu_command.c | 3 +--
> src/qemu/qemu_firmware.c | 4 +---
> src/qemu/qemu_migration_params.c | 4 +---
> src/qemu/qemu_monitor_json.c | 11 +++--------
> src/rpc/virnetserver.c | 6 ++----
> src/rpc/virnetserverservice.c | 3 +--
> src/util/virjson.c | 12 +++++-------
> src/util/virjson.h | 3 ++-
> src/util/virlease.c | 2 +-
> src/util/virlockspace.c | 6 ++----
> src/util/virmacmap.c | 6 ++----
> tests/testutilsqemuschema.c | 3 +--
> 18 files changed, 32 insertions(+), 65 deletions(-)
I think the following should be squashed in:
diff --git i/src/qemu/qemu_monitor_json.c w/src/qemu/qemu_monitor_json.c
index 924e03b4da..2e040a2bc1 100644
--- i/src/qemu/qemu_monitor_json.c
+++ w/src/qemu/qemu_monitor_json.c
@@ -7580,7 +7580,6 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
cleanup:
VIR_FREE(tlsalias);
- virJSONValueFree(addr);
virJSONValueFree(data);
virJSONValueFree(backend);
return ret;
The value is stolen on success only, so we must free it explictly, or
it can be separately converted to converted to g_autoptr.
6:55 p.m.
New subject: [PATCH 21/25] qemuMonitorJSONTransactionAdd: Refactor cleanup
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 94365c775b..76401c4d3c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -510,31 +510,28 @@ qemuMonitorJSONTransactionAdd(virJSONValuePtr actions,
const char *cmdname,
...)
{
- virJSONValuePtr entry = NULL;
- virJSONValuePtr data = NULL;
+ g_autoptr(virJSONValue) entry = NULL;
+ g_autoptr(virJSONValue) data = NULL;
va_list args;
- int ret = -1;
va_start(args, cmdname);
- if (virJSONValueObjectCreateVArgs(&data, args) < 0)
- goto cleanup;
+ if (virJSONValueObjectCreateVArgs(&data, args) < 0) {
+ va_end(args);
+ return -1;
+ }
+
+ va_end(args);
if (virJSONValueObjectCreate(&entry,
"s:type", cmdname,
"A:data", &data, NULL) < 0)
- goto cleanup;
+ return -1;
if (virJSONValueArrayAppend(actions, &entry) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- virJSONValueFree(entry);
- virJSONValueFree(data);
- va_end(args);
- return ret;
+ return 0;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 22/25] qemuAgentSetVCPUsCommand: Refactor cleanup
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_agent.c | 39 +++++++++++++--------------------------
1 file changed, 13 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 9aec0fdb4b..9a74b802b8 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1483,20 +1483,17 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr agent,
size_t ninfo,
int *nmodified)
{
- int ret = -1;
- virJSONValuePtr cmd = NULL;
- virJSONValuePtr reply = NULL;
- virJSONValuePtr cpus = NULL;
- virJSONValuePtr cpu = NULL;
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
+ g_autoptr(virJSONValue) cpus = virJSONValueNewArray();
size_t i;
+ int ret;
*nmodified = 0;
- /* create the key data array */
- cpus = virJSONValueNewArray();
-
for (i = 0; i < ninfo; i++) {
qemuAgentCPUInfoPtr in = &info[i];
+ g_autoptr(virJSONValue) cpu = virJSONValueNewObject();
/* don't set state for cpus that were not touched */
if (!in->modified)
@@ -1504,31 +1501,26 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr agent,
(*nmodified)++;
- /* create single cpu object */
- cpu = virJSONValueNewObject();
-
if (virJSONValueObjectAppendNumberInt(cpu, "logical-id", in->id)
< 0)
- goto cleanup;
+ return -1;
if (virJSONValueObjectAppendBoolean(cpu, "online", in->online) <
0)
- goto cleanup;
+ return -1;
if (virJSONValueArrayAppend(cpus, &cpu) < 0)
- goto cleanup;
+ return -1;
}
- if (*nmodified == 0) {
- ret = 0;
- goto cleanup;
- }
+ if (*nmodified == 0)
+ return 0;
if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus",
"a:vcpus", &cpus,
NULL)))
- goto cleanup;
+ return -1;
if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
- goto cleanup;
+ return -1;
/* All negative values are invalid. Return of 0 is bogus since we wouldn't
* call the guest agent so that 0 cpus would be set successfully. Reporting
@@ -1537,14 +1529,9 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr agent,
ret <= 0 || ret > *nmodified) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest agent returned malformed or invalid return
value"));
- ret = -1;
+ return -1;
}
- cleanup:
- virJSONValueFree(cmd);
- virJSONValueFree(reply);
- virJSONValueFree(cpu);
- virJSONValueFree(cpus);
return ret;
}
--
2.29.2
6:55 p.m.
New subject: [PATCH 23/25] virJSONParserHandle*: Refactor memory cleanup and drop NULL checks
virJSONValueNew* won't return error nowadays so NULL checks are not
necessary. The memory can be cleared via g_autoptr.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 68 ++++++++++++++++------------------------------
1 file changed, 24 insertions(+), 44 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 65215f03c5..de83441e27 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1604,17 +1604,13 @@ static int
virJSONParserHandleNull(void *ctx)
{
virJSONParserPtr parser = ctx;
- virJSONValuePtr value = virJSONValueNewNull();
+ g_autoptr(virJSONValue) value = virJSONValueNewNull();
VIR_DEBUG("parser=%p", parser);
- if (!value)
+ if (virJSONParserInsertValue(parser, value) < 0)
return 0;
-
- if (virJSONParserInsertValue(parser, value) < 0) {
- virJSONValueFree(value);
- return 0;
- }
+ value = NULL;
return 1;
}
@@ -1625,17 +1621,13 @@ virJSONParserHandleBoolean(void *ctx,
int boolean_)
{
virJSONParserPtr parser = ctx;
- virJSONValuePtr value = virJSONValueNewBoolean(boolean_);
+ g_autoptr(virJSONValue) value = virJSONValueNewBoolean(boolean_);
VIR_DEBUG("parser=%p boolean=%d", parser, boolean_);
- if (!value)
- return 0;
-
- if (virJSONParserInsertValue(parser, value) < 0) {
- virJSONValueFree(value);
+ if (virJSONParserInsertValue(parser, value) < 0)
return 0;
- }
+ value = NULL;
return 1;
}
@@ -1647,22 +1639,14 @@ virJSONParserHandleNumber(void *ctx,
size_t l)
{
virJSONParserPtr parser = ctx;
- char *str;
- virJSONValuePtr value;
-
- str = g_strndup(s, l);
- value = virJSONValueNewNumber(str);
- VIR_FREE(str);
+ g_autofree char *str = g_strndup(s, l);
+ g_autoptr(virJSONValue) value = virJSONValueNewNumber(str);
VIR_DEBUG("parser=%p str=%s", parser, str);
- if (!value)
+ if (virJSONParserInsertValue(parser, value) < 0)
return 0;
-
- if (virJSONParserInsertValue(parser, value) < 0) {
- virJSONValueFree(value);
- return 0;
- }
+ value = NULL;
return 1;
}
@@ -1674,18 +1658,14 @@ virJSONParserHandleString(void *ctx,
size_t stringLen)
{
virJSONParserPtr parser = ctx;
- virJSONValuePtr value = virJSONValueNewStringLen((const char *)stringVal,
- stringLen);
+ g_autoptr(virJSONValue) value = virJSONValueNewStringLen((const char *)stringVal,
+ stringLen);
VIR_DEBUG("parser=%p str=%p", parser, (const char *)stringVal);
- if (!value)
- return 0;
-
- if (virJSONParserInsertValue(parser, value) < 0) {
- virJSONValueFree(value);
+ if (virJSONParserInsertValue(parser, value) < 0)
return 0;
- }
+ value = NULL;
return 1;
}
@@ -1716,21 +1696,21 @@ static int
virJSONParserHandleStartMap(void *ctx)
{
virJSONParserPtr parser = ctx;
- virJSONValuePtr value = virJSONValueNewObject();
+ g_autoptr(virJSONValue) value = virJSONValueNewObject();
+ virJSONValuePtr tmp = value;
VIR_DEBUG("parser=%p", parser);
- if (virJSONParserInsertValue(parser, value) < 0) {
- virJSONValueFree(value);
+ if (virJSONParserInsertValue(parser, value) < 0)
return 0;
- }
+ value = NULL;
if (VIR_REALLOC_N(parser->state,
parser->nstate + 1) < 0) {
return 0;
}
- parser->state[parser->nstate].value = value;
+ parser->state[parser->nstate].value = tmp;
parser->state[parser->nstate].key = NULL;
parser->nstate++;
@@ -1765,20 +1745,20 @@ static int
virJSONParserHandleStartArray(void *ctx)
{
virJSONParserPtr parser = ctx;
- virJSONValuePtr value = virJSONValueNewArray();
+ g_autoptr(virJSONValue) value = virJSONValueNewArray();
+ virJSONValuePtr tmp = value;
VIR_DEBUG("parser=%p", parser);
- if (virJSONParserInsertValue(parser, value) < 0) {
- virJSONValueFree(value);
+ if (virJSONParserInsertValue(parser, value) < 0)
return 0;
- }
+ value = NULL;
if (VIR_REALLOC_N(parser->state,
parser->nstate + 1) < 0)
return 0;
- parser->state[parser->nstate].value = value;
+ parser->state[parser->nstate].value = tmp;
parser->state[parser->nstate].key = NULL;
parser->nstate++;
--
2.29.2
6:55 p.m.
New subject: [PATCH 24/25] virJSONValueNewNumber: Take ownership of passed string
Avoid pointless copies of temporary strings when constructing number
JSON objects.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index de83441e27..b21b1fc63f 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -465,15 +465,22 @@ virJSONValueNewStringLen(const char *data,
}
+/**
+ * virJSONValueNewNumber:
+ * @data: string representing the number
+ *
+ * Creates a new virJSONValue of VIR_JSON_TYPE_NUMBER type. Note that this
+ * function takes ownership of @data.
+ */
static virJSONValuePtr
-virJSONValueNewNumber(const char *data)
+virJSONValueNewNumber(char *data)
{
virJSONValuePtr val;
val = g_new0(virJSONValue, 1);
val->type = VIR_JSON_TYPE_NUMBER;
- val->data.number = g_strdup(data);
+ val->data.number = data;
return val;
}
@@ -482,43 +489,35 @@ virJSONValueNewNumber(const char *data)
virJSONValuePtr
virJSONValueNewNumberInt(int data)
{
- g_autofree char *str = NULL;
- str = g_strdup_printf("%i", data);
- return virJSONValueNewNumber(str);
+ return virJSONValueNewNumber(g_strdup_printf("%i", data));
}
virJSONValuePtr
virJSONValueNewNumberUint(unsigned int data)
{
- g_autofree char *str = NULL;
- str = g_strdup_printf("%u", data);
- return virJSONValueNewNumber(str);
+ return virJSONValueNewNumber(g_strdup_printf("%u", data));
}
virJSONValuePtr
virJSONValueNewNumberLong(long long data)
{
- g_autofree char *str = NULL;
- str = g_strdup_printf("%lld", data);
- return virJSONValueNewNumber(str);
+ return virJSONValueNewNumber(g_strdup_printf("%lld", data));
}
virJSONValuePtr
virJSONValueNewNumberUlong(unsigned long long data)
{
- g_autofree char *str = NULL;
- str = g_strdup_printf("%llu", data);
- return virJSONValueNewNumber(str);
+ return virJSONValueNewNumber(g_strdup_printf("%llu", data));
}
virJSONValuePtr
virJSONValueNewNumberDouble(double data)
{
- g_autofree char *str = NULL;
+ char *str = NULL;
if (virDoubleToStr(&str, data) < 0)
return NULL;
return virJSONValueNewNumber(str);
@@ -1534,7 +1533,7 @@ virJSONValueCopy(const virJSONValue *in)
out = virJSONValueNewString(in->data.string);
break;
case VIR_JSON_TYPE_NUMBER:
- out = virJSONValueNewNumber(in->data.number);
+ out = virJSONValueNewNumber(g_strdup(in->data.number));
break;
case VIR_JSON_TYPE_BOOLEAN:
out = virJSONValueNewBoolean(in->data.boolean);
@@ -1639,10 +1638,9 @@ virJSONParserHandleNumber(void *ctx,
size_t l)
{
virJSONParserPtr parser = ctx;
- g_autofree char *str = g_strndup(s, l);
- g_autoptr(virJSONValue) value = virJSONValueNewNumber(str);
+ g_autoptr(virJSONValue) value = virJSONValueNewNumber(g_strndup(s, l));
- VIR_DEBUG("parser=%p str=%s", parser, str);
+ VIR_DEBUG("parser=%p str=%s", parser, value->data.number);
if (virJSONParserInsertValue(parser, value) < 0)
return 0;
--
2.29.2
6:55 p.m.
New subject: [PATCH 25/25] virJSONParserInsertValue: Take double pointer for @value
The function calls virJSONValueObjectAppend/virJSONValueArrayAppend, so
by taking a double pointer we can drop the pointer clearing from
callers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virjson.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index b21b1fc63f..b952ad500d 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1550,10 +1550,10 @@ virJSONValueCopy(const virJSONValue *in)
#if WITH_YAJL
static int
virJSONParserInsertValue(virJSONParserPtr parser,
- virJSONValuePtr value)
+ virJSONValuePtr *value)
{
if (!parser->head) {
- parser->head = value;
+ parser->head = g_steal_pointer(value);
} else {
virJSONParserStatePtr state;
if (!parser->nstate) {
@@ -1572,7 +1572,7 @@ virJSONParserInsertValue(virJSONParserPtr parser,
if (virJSONValueObjectAppend(state->value,
state->key,
- &value) < 0)
+ value) < 0)
return -1;
VIR_FREE(state->key);
@@ -1585,7 +1585,7 @@ virJSONParserInsertValue(virJSONParserPtr parser,
}
if (virJSONValueArrayAppend(state->value,
- &value) < 0)
+ value) < 0)
return -1;
} break;
@@ -1607,9 +1607,8 @@ virJSONParserHandleNull(void *ctx)
VIR_DEBUG("parser=%p", parser);
- if (virJSONParserInsertValue(parser, value) < 0)
+ if (virJSONParserInsertValue(parser, &value) < 0)
return 0;
- value = NULL;
return 1;
}
@@ -1624,9 +1623,8 @@ virJSONParserHandleBoolean(void *ctx,
VIR_DEBUG("parser=%p boolean=%d", parser, boolean_);
- if (virJSONParserInsertValue(parser, value) < 0)
+ if (virJSONParserInsertValue(parser, &value) < 0)
return 0;
- value = NULL;
return 1;
}
@@ -1642,9 +1640,8 @@ virJSONParserHandleNumber(void *ctx,
VIR_DEBUG("parser=%p str=%s", parser, value->data.number);
- if (virJSONParserInsertValue(parser, value) < 0)
+ if (virJSONParserInsertValue(parser, &value) < 0)
return 0;
- value = NULL;
return 1;
}
@@ -1661,9 +1658,8 @@ virJSONParserHandleString(void *ctx,
VIR_DEBUG("parser=%p str=%p", parser, (const char *)stringVal);
- if (virJSONParserInsertValue(parser, value) < 0)
+ if (virJSONParserInsertValue(parser, &value) < 0)
return 0;
- value = NULL;
return 1;
}
@@ -1699,9 +1695,8 @@ virJSONParserHandleStartMap(void *ctx)
VIR_DEBUG("parser=%p", parser);
- if (virJSONParserInsertValue(parser, value) < 0)
+ if (virJSONParserInsertValue(parser, &value) < 0)
return 0;
- value = NULL;
if (VIR_REALLOC_N(parser->state,
parser->nstate + 1) < 0) {
@@ -1748,9 +1743,8 @@ virJSONParserHandleStartArray(void *ctx)
VIR_DEBUG("parser=%p", parser);
- if (virJSONParserInsertValue(parser, value) < 0)
+ if (virJSONParserInsertValue(parser, &value) < 0)
return 0;
- value = NULL;
if (VIR_REALLOC_N(parser->state,
parser->nstate + 1) < 0)
--
2.29.2
10:11 a.m.
On 2/12/21 6:55 PM, Peter Krempa wrote:
Peter Krempa (25):
virLockDaemonPreExecRestart: Refactor memory cleanup
virLogDaemonPreExecRestart: Refactor memory cleanup
virLogHandlerPreExecRestart: Refactor memory cleanup
virNetDaemonPreExecRestart: Refactor memory cleanup
virNetServerServicePreExecRestart: Refactor memory cleanup
virNetServerClientPreExecRestart: Refactor memory cleanup
virNetServerPreExecRestart: Drop error reporting from
virJSONValueObjectAppend* calls
virNetServerPreExecRestart: Refactor memory cleanup
virLockSpacePreExecRestart: Refactor memory cleanup
qemuAgentMakeCommand: Refactor memory cleanup
virJSONValueObjectInsert: Clear @value on successful insertion
virJSONValueCopy: Don't use virJSONValue(Object|Array)Append
virJSONValue(Array|Object)Append*: Simplify handling of appended
object
virJSONValueNewArrayFromBitmap: Refactor cleanup
virJSONValueObjectAddVArgs: Use autofree for the temporary bitmap
virJSONValueObjectAppend: Clear pointer when taking ownership of
passed value
qemuAgentMakeStringsArray: Refactor cleanup
virMACMapHashDumper: Refactor array addition
testQEMUSchemaValidateObjectMergeVariantMember: Fix theoretical leak
virJSONValueArrayAppend: Clear pointer when taking ownership of passed
value
qemuMonitorJSONTransactionAdd: Refactor cleanup
qemuAgentSetVCPUsCommand: Refactor cleanup
virJSONParserHandle*: Refactor memory cleanup and drop NULL checks
virJSONValueNewNumber: Take ownership of passed string
virJSONParserInsertValue: Take double pointer for @value
src/locking/lock_daemon.c | 77 ++++----
src/logging/log_daemon.c | 52 +++---
src/logging/log_handler.c | 40 ++--
src/network/leaseshelper.c | 3 +-
src/node_device/node_device_driver.c | 4 +-
src/qemu/qemu_agent.c | 89 ++++-----
src/qemu/qemu_block.c | 19 +-
src/qemu/qemu_command.c | 3 +-
src/qemu/qemu_firmware.c | 27 +--
src/qemu/qemu_migration_params.c | 4 +-
src/qemu/qemu_monitor_json.c | 58 +++---
src/rpc/virnetdaemon.c | 25 +--
src/rpc/virnetserver.c | 79 +++-----
src/rpc/virnetserverclient.c | 24 +--
src/rpc/virnetserverservice.c | 34 ++--
src/util/virjson.c | 261 +++++++++++----------------
src/util/virjson.h | 7 +-
src/util/virlease.c | 2 +-
src/util/virlockspace.c | 47 ++---
src/util/virmacmap.c | 13 +-
tests/testutilsqemuschema.c | 4 +-
21 files changed, 322 insertions(+), 550 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1304
days inactive
1312
days old
28 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa