On Thu, May 10, 2018 at 11:43:11AM -0400, John Ferlan wrote:
Rather than have virJSONValueArraySize return a -1 when the input is not an array and then splat an error message, let's check for an array before calling and then change the return to be a size_t instead of ssize_t. That means using the helper virJSONValueIsArray as well as using a more generic error message such as "Malformed <something> array". In some cases we can remove stack variables and when we cannot, those variables should be size_t not ssize_t. Alter a few references of if (!value) to be if (value == 0) instead as well. Some callers can already assume an array is being worked on based on the previous call, so there's less to do. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- Original: https://www.redhat.com/archives/libvir-list/2018-April/msg02003.html Reposting as I found a recent upstream changes created some simple merge conflicts and it's never clear if applying what was there can be applied on top even with git am -3, so I figured I would just repost. As noted previously, based off a review of the Coverity work from Peter: https://www.redhat.com/archives/libvir-list/2018-April/msg01613.html src/locking/lock_daemon.c | 7 ++-- src/logging/log_handler.c | 7 ++-- src/network/bridge_driver.c | 7 ++-- src/qemu/qemu_agent.c | 35 ++++++++++-------- src/qemu/qemu_monitor_json.c | 85 ++++++++++++++++++++----------------------- src/rpc/virnetdaemon.c | 7 +--- src/rpc/virnetserver.c | 15 +++----- src/rpc/virnetserverservice.c | 7 ++-- src/util/virjson.c | 5 +-- src/util/virjson.h | 2 +- src/util/virlockspace.c | 16 ++++---- src/util/virstoragefile.c | 3 +- tests/testutilsqemuschema.c | 18 +++------ tools/nss/libvirt_nss.c | 3 +- 14 files changed, 97 insertions(+), 120 deletions(-) @@ -1869,15 +1875,14 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, goto cleanup; } - if (virJSONValueGetType(data) != VIR_JSON_TYPE_ARRAY) { + if (!virJSONValueIsArray(data)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest-get-fsinfo return information was not " - "an array")); + _("Malformed guest-get-fsinfo data array")); goto cleanup; } ndata = virJSONValueArraySize(data); - if (!ndata) { + if (ndata == 0) { ret = 0; *info = NULL; goto cleanup; @@ -1928,14 +1933,14 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, goto cleanup; } - if (virJSONValueGetType(entry) != VIR_JSON_TYPE_ARRAY) { + if (!virJSONValueIsArray(entry)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest-get-fsinfo 'disk' data was not an array")); + _("Malformed guest-get-fsinfo 'disk' data array")); goto cleanup; } ndisk = virJSONValueArraySize(entry); - if (!ndisk) + if (ndisk == 0) continue; if (VIR_ALLOC_N(info_ret[i]->devAlias, ndisk) < 0) goto cleanup;
Both ndata and ndisk should be switched to size_t in this function. With that change: Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx> Jano
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
