[PATCH 00/25] Clear pointers in virJSONValue(Object|Array)Append and other cleanups
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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
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@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa <pkrempa@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
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@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
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@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
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@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
Use g_autoptr for the JSON value objects and remove the cleanup label and inline freeing of objects. Signed-off-by: Peter Krempa <pkrempa@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
Signed-off-by: Peter Krempa <pkrempa@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
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@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
Signed-off-by: Peter Krempa <pkrempa@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
Use automatic memory freeing and don't check return value of virJSONValueNewString as it can't fail. Signed-off-by: Peter Krempa <pkrempa@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
Signed-off-by: Peter Krempa <pkrempa@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
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@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
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@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
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@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.
Signed-off-by: Peter Krempa <pkrempa@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
Signed-off-by: Peter Krempa <pkrempa@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
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@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
Avoid pointless copies of temporary strings when constructing number JSON objects. Signed-off-by: Peter Krempa <pkrempa@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
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@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
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@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa