[libvirt] [PATCH 0/9] Remove remaining usage of VIR_STR(N)DUP (glib chronicles)

Almost. Patch 8/9 depends on Jirka removing VIR_STRNDUP in [PATCH v3 04/52] conf: Drop nameLen parameter from virDomainCapsCPUModelsAdd https://www.redhat.com/archives/libvir-list/2019-November/msg00068.html Ján Tomko (9): Remove VIR_STRDUP usage that snuck in Remove VIR_STRNDUP usage that passes -1 Remove VIR_STRNDUP usage that subtracts from a non-NULL pointer Remove VIR_STRNDUP usage with checked pointers Remove VIR_STRNDUP usage with subtraction Remove the rest of VIR_STRNDUP tests: delete tests for VIR_STR(N)DUP util: remove VIR_STRDUP and VIR_STRNDUP docs: hacking: document removal of VIR_STR(N)DUP docs/hacking.html.in | 10 +- src/bhyve/bhyve_parse_command.c | 44 +++-- src/conf/nwfilter_conf.c | 4 +- src/conf/nwfilter_params.c | 6 +- src/interface/interface_backend_udev.c | 8 +- src/libvirt_private.syms | 2 - src/libxl/xen_common.c | 44 +++-- src/libxl/xen_xl.c | 6 +- src/libxl/xen_xm.c | 9 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_domain.c | 9 +- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_monitor_json.c | 11 +- src/remote/remote_driver.c | 6 +- src/rpc/virnetlibsshsession.c | 7 +- src/storage/storage_backend_logical.c | 7 +- src/util/vircgroupv1.c | 5 +- src/util/vircommand.c | 5 +- src/util/virconf.c | 24 +-- src/util/viriscsi.c | 3 +- src/util/virjson.c | 11 +- src/util/virkeyfile.c | 9 +- src/util/virsocketaddr.c | 5 +- src/util/virstoragefile.c | 6 +- src/util/virstring.c | 56 +----- src/util/virstring.h | 69 ------- src/util/virsysinfo.c | 244 ++++++++++++------------- src/vmware/vmware_conf.c | 6 +- tests/virstringtest.c | 136 -------------- tools/vsh.c | 5 +- 30 files changed, 218 insertions(+), 545 deletions(-) -- 2.21.0

Fixes: 224d269f19f0a6c496dd2218f934a54742d51708 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9c517597bb..ffac73dd42 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -12835,8 +12835,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, if (!usb) goto cleanup; - if (VIR_STRDUP(tmpPath, virUSBDeviceGetPath(usb)) < 0) - goto cleanup; + tmpPath = g_strdup(virUSBDeviceGetPath(usb)); perm = VIR_CGROUP_DEVICE_RW; break; @@ -12857,8 +12856,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, if (!scsi) goto cleanup; - if (VIR_STRDUP(tmpPath, virSCSIDeviceGetPath(scsi)) < 0) - goto cleanup; + tmpPath = g_strdup(virSCSIDeviceGetPath(scsi)); perm = virSCSIDeviceGetReadonly(scsi) ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW; } @@ -12870,8 +12868,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, if (!(host = virSCSIVHostDeviceNew(hostsrc->wwpn))) goto cleanup; - if (VIR_STRDUP(tmpPath, virSCSIVHostDeviceGetPath(host)) < 0) - goto cleanup; + tmpPath = g_strdup(virSCSIVHostDeviceGetPath(host)); perm = VIR_CGROUP_DEVICE_RW; } break; -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Fixes: 224d269f19f0a6c496dd2218f934a54742d51708
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Replace all the usage of VIR_STRNDUP(dest, b, p ? p - b : -1) with separate calls to g_strndup/g_strdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/bhyve/bhyve_parse_command.c | 38 +++++++++++++++++++++------------ src/libxl/xen_common.c | 15 +++++++------ src/remote/remote_driver.c | 6 ++++-- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index fa3b881f21..5c5dfae4a9 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -150,8 +150,10 @@ bhyveCommandLineToArgv(const char *nativeConfig, start = curr; next = strchr(curr, '\n'); - if (VIR_STRNDUP(line, curr, next ? next - curr : -1) < 0) - goto error; + if (next) + line = g_strndup(curr, next - curr); + else + line = g_strdup(curr); if (VIR_RESIZE_N(lines, lines_alloc, line_count, 2) < 0) { VIR_FREE(line); @@ -194,8 +196,10 @@ bhyveCommandLineToArgv(const char *nativeConfig, next = strchr(start, ' '); } - if (VIR_STRNDUP(arg, curr, next ? next - curr : -1) < 0) - goto error; + if (next) + arg = g_strndup(curr, next - curr); + else + arg = g_strdup(curr); if (next && (*next == '\'' || *next == '"')) next++; @@ -366,8 +370,10 @@ bhyveParsePCISlot(const char *slotdef, next = strchr(curr, ':'); - if (VIR_STRNDUP(val, curr, next? next - curr : -1) < 0) - goto error; + if (next) + val = g_strndup(curr, next - curr); + else + val = g_strdup(curr); if (virStrToLong_ui(val, NULL, 10, &values[i]) < 0) goto error; @@ -441,9 +447,10 @@ bhyveParsePCIDisk(virDomainDefPtr def, goto error; separator = strchr(config, ','); - if (VIR_STRNDUP(disk->src->path, config, - separator? separator - config : -1) < 0) - goto error; + if (separator) + disk->src->path = g_strndup(config, separator - config); + else + disk->src->path = g_strdup(config); if (bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { idx = *nvirtiodisk; @@ -515,9 +522,10 @@ bhyveParsePCINet(virDomainDefPtr def, } separator = strchr(config, ','); - if (VIR_STRNDUP(net->ifname, config, - separator? separator - config : -1) < 0) - goto error; + if (separator) + net->ifname = g_strndup(config, separator - config); + else + net->ifname = g_strdup(config); if (!separator) goto cleanup; @@ -578,8 +586,10 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def, if (conf) conf++; /* Skip initial comma */ - if (VIR_STRNDUP(emulation, separator, conf? conf - separator - 1 : -1) < 0) - goto error; + if (conf) + emulation = g_strndup(separator, conf - separator - 1); + else + emulation = g_strdup(separator); if (bhyveParsePCISlot(slotdef, &pcislot, &bus, &function) < 0) goto error; diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index c31a5c952e..a4a9ec59bf 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -823,9 +823,11 @@ xenParseSxprChar(const char *value, offset2 = strchr(offset, ','); offset++; - if (VIR_STRNDUP(def->source->data.tcp.service, offset, - offset2 ? offset2 - offset : -1) < 0) - goto error; + if (offset2) + def->source->data.tcp.service = g_strndup(offset, + offset2 - offset); + else + def->source->data.tcp.service = g_strdup(offset); if (offset2 && strstr(offset2, ",server")) def->source->data.tcp.listen = true; @@ -875,9 +877,10 @@ xenParseSxprChar(const char *value, case VIR_DOMAIN_CHR_TYPE_UNIX: { const char *offset = strchr(value, ','); - if (VIR_STRNDUP(def->source->data.nix.path, value, - offset ? offset - value : -1) < 0) - goto error; + if (offset) + def->source->data.nix.path = g_strndup(value, offset - value); + else + def->source->data.nix.path = g_strdup(value); if (offset != NULL && strstr(offset, ",server") != NULL) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 451a45f590..503f49a902 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -215,8 +215,10 @@ static int remoteSplitURIScheme(virURIPtr uri, *driver = *transport = NULL; - if (VIR_STRNDUP(*driver, uri->scheme, p ? p - uri->scheme : -1) < 0) - return -1; + if (p) + *driver = g_strndup(uri->scheme, p - uri->scheme); + else + *driver = g_strdup(uri->scheme); if (p) { *transport = g_strdup(p + 1); -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Replace all the usage of VIR_STRNDUP(dest, b, p ? p - b : -1) with separate calls to g_strndup/g_strdup.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/bhyve/bhyve_parse_command.c | 38 +++++++++++++++++++++------------ src/libxl/xen_common.c | 15 +++++++------ src/remote/remote_driver.c | 6 ++++-- 3 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index fa3b881f21..5c5dfae4a9 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -150,8 +150,10 @@ bhyveCommandLineToArgv(const char *nativeConfig, start = curr; next = strchr(curr, '\n');
- if (VIR_STRNDUP(line, curr, next ? next - curr : -1) < 0) - goto error; + if (next) + line = g_strndup(curr, next - curr); + else + line = g_strdup(curr);
if (VIR_RESIZE_N(lines, lines_alloc, line_count, 2) < 0) { VIR_FREE(line); @@ -194,8 +196,10 @@ bhyveCommandLineToArgv(const char *nativeConfig, next = strchr(start, ' '); }
- if (VIR_STRNDUP(arg, curr, next ? next - curr : -1) < 0) - goto error; + if (next) + arg = g_strndup(curr, next - curr); + else + arg = g_strdup(curr);
if (next && (*next == '\'' || *next == '"')) next++; @@ -366,8 +370,10 @@ bhyveParsePCISlot(const char *slotdef,
next = strchr(curr, ':');
- if (VIR_STRNDUP(val, curr, next? next - curr : -1) < 0) - goto error; + if (next) + val = g_strndup(curr, next - curr); + else + val = g_strdup(curr);
if (virStrToLong_ui(val, NULL, 10, &values[i]) < 0) goto error; @@ -441,9 +447,10 @@ bhyveParsePCIDisk(virDomainDefPtr def, goto error;
separator = strchr(config, ','); - if (VIR_STRNDUP(disk->src->path, config, - separator? separator - config : -1) < 0) - goto error; + if (separator) + disk->src->path = g_strndup(config, separator - config); + else + disk->src->path = g_strdup(config);
if (bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { idx = *nvirtiodisk; @@ -515,9 +522,10 @@ bhyveParsePCINet(virDomainDefPtr def, }
separator = strchr(config, ','); - if (VIR_STRNDUP(net->ifname, config, - separator? separator - config : -1) < 0) - goto error; + if (separator) + net->ifname = g_strndup(config, separator - config); + else + net->ifname = g_strdup(config);
if (!separator) goto cleanup; @@ -578,8 +586,10 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def, if (conf) conf++; /* Skip initial comma */
- if (VIR_STRNDUP(emulation, separator, conf? conf - separator - 1 : -1) < 0) - goto error; + if (conf) + emulation = g_strndup(separator, conf - separator - 1); + else + emulation = g_strdup(separator);
Nit: there's an 'if (conf)' right before the VIR_STRNDUP() call you're replacing. You can get a ride in it to insert the g_strndup call instead of adding a new 'if (conf)': if (conf) { conf++; /* Skip initial comma */ emulation = g_strndup(separator, conf - separator - 1); } else { emulation = g_strdup(separator); } Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Use g_strndup in all the cases where we check upfront whether a pointer is non-NULL and then use it to calculate the copied length. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virsysinfo.c | 241 ++++++++++++++++++++---------------------- 1 file changed, 112 insertions(+), 129 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 9d013067fe..635e4c4bfb 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -220,23 +220,23 @@ virSysinfoParsePPCSystem(const char *base, virSysinfoSystemDefPtr *sysdef) cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->family = g_strndup(cur, eol - cur); if ((cur = strstr(base, "model")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->serial = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "machine")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->version = g_strndup(cur, eol - cur); } if (!def->manufacturer && !def->product && !def->version && @@ -248,7 +248,6 @@ virSysinfoParsePPCSystem(const char *base, virSysinfoSystemDefPtr *sysdef) *sysdef = def; def = NULL; ret = 0; - cleanup: virSysinfoSystemDefFree(def); return ret; } @@ -270,18 +269,17 @@ virSysinfoParsePPCProcessor(const char *base, virSysinfoDefPtr ret) processor = &ret->processor[ret->nprocessor - 1]; virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(processor->processor_socket_destination, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_socket_destination = g_strndup(cur, + eol - cur); base = cur; if ((cur = strstr(base, "cpu")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(processor->processor_type, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_type = g_strndup(cur, eol - cur); base = cur; } @@ -289,9 +287,8 @@ virSysinfoParsePPCProcessor(const char *base, virSysinfoDefPtr ret) cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(processor->processor_version, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_version = g_strndup(cur, eol - cur); base = cur; } @@ -354,23 +351,23 @@ virSysinfoParseARMSystem(const char *base, virSysinfoSystemDefPtr *sysdef) cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->family = g_strndup(cur, eol - cur); if ((cur = strstr(base, "model")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->serial = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "machine")) != NULL) { cur = strchr(cur, ':') + 1; eol = strchr(cur, '\n'); virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->version = g_strndup(cur, eol - cur); } if (!def->manufacturer && !def->product && !def->version && @@ -382,7 +379,6 @@ virSysinfoParseARMSystem(const char *base, virSysinfoSystemDefPtr *sysdef) *sysdef = def; def = NULL; ret = 0; - cleanup: virSysinfoSystemDefFree(def); return ret; } @@ -402,8 +398,8 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) eol = strchr(tmp_base, '\n'); cur = strchr(tmp_base, ':') + 1; virSkipSpaces(&cur); - if (eol && VIR_STRNDUP(processor_type, cur, eol - cur) < 0) - goto error; + if (eol) + processor_type = g_strndup(cur, eol - cur); while ((tmp_base = strstr(base, "processor")) != NULL) { base = tmp_base; @@ -415,10 +411,9 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) processor = &ret->processor[ret->nprocessor - 1]; virSkipSpaces(&cur); - if (eol && - VIR_STRNDUP(processor->processor_socket_destination, - cur, eol - cur) < 0) - goto error; + if (eol) + processor->processor_socket_destination = g_strndup(cur, + eol - cur); processor->processor_type = g_strdup(processor_type); @@ -670,29 +665,29 @@ virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios) cur += 8; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->vendor, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->vendor = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->version = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Release Date: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->date, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->date = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "BIOS Revision: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->release, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->release = g_strndup(cur, eol - cur); } if (!def->vendor && !def->version && @@ -704,7 +699,6 @@ virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios) *bios = def; def = NULL; ret = 0; - cleanup: virSysinfoBIOSDefFree(def); return ret; } @@ -728,50 +722,50 @@ virSysinfoParseX86System(const char *base, virSysinfoSystemDefPtr *sysdef) cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->manufacturer = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Product Name: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->product = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->version = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->serial = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "UUID: ")) != NULL) { cur += 6; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->uuid, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->uuid = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "SKU Number: ")) != NULL) { cur += 12; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->sku, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->sku = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Family: ")) != NULL) { cur += 8; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->family, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->family = g_strndup(cur, eol - cur); } if (!def->manufacturer && !def->product && !def->version && @@ -783,7 +777,6 @@ virSysinfoParseX86System(const char *base, virSysinfoSystemDefPtr *sysdef) *sysdef = def; def = NULL; ret = 0; - cleanup: virSysinfoSystemDefFree(def); return ret; } @@ -813,43 +806,43 @@ virSysinfoParseX86BaseBoard(const char *base, cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->manufacturer = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Product Name: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->product, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->product = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->version = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->serial = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Asset Tag: ")) != NULL) { cur += 11; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->asset = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Location In Chassis: ")) != NULL) { cur += 21; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->location, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->location = g_strndup(cur, eol - cur); } if (!def->manufacturer && !def->product && !def->version && @@ -898,36 +891,36 @@ virSysinfoParseX86Chassis(const char *base, cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->manufacturer, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->manufacturer = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->version, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->version = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->serial, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->serial = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Asset Tag: ")) != NULL) { cur += 11; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->asset, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->asset = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "SKU Number: ")) != NULL) { cur += 12; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(def->sku, cur, eol - cur) < 0) - goto cleanup; + if (eol) + def->sku = g_strndup(cur, eol - cur); } if (!def->manufacturer && !def->version && @@ -939,7 +932,6 @@ virSysinfoParseX86Chassis(const char *base, *chassisdef = def; def = NULL; ret = 0; - cleanup: virSysinfoChassisDefFree(def); return ret; } @@ -964,86 +956,80 @@ virSysinfoParseX86Processor(const char *base, virSysinfoDefPtr ret) cur += 20; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_socket_destination, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_socket_destination = g_strndup(cur, + eol - cur); } if ((cur = strstr(base, "Type: ")) != NULL) { cur += 6; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_type, cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_type = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Family: ")) != NULL) { cur += 8; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_family, cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_family = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Manufacturer: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_manufacturer, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_manufacturer = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Signature: ")) != NULL) { cur += 11; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_signature, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_signature = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Version: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_version, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_version = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "External Clock: ")) != NULL) { cur += 16; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_external_clock, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_external_clock = g_strndup(cur, + eol - cur); } if ((cur = strstr(base, "Max Speed: ")) != NULL) { cur += 11; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_max_speed, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_max_speed = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Status: ")) != NULL) { cur += 8; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_status, cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_status = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_serial_number, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_serial_number = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Part Number: ")) != NULL) { cur += 13; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(processor->processor_part_number, - cur, eol - cur) < 0) - return -1; + if (eol) + processor->processor_part_number = g_strndup(cur, eol - cur); } base += strlen("Processor Information"); @@ -1074,74 +1060,71 @@ virSysinfoParseX86Memory(const char *base, virSysinfoDefPtr ret) goto next; virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_size, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_size = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Form Factor: ")) != NULL) { cur += 13; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_form_factor, - cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_form_factor = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Locator: ")) != NULL) { cur += 9; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_locator, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_locator = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Bank Locator: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_bank_locator, - cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_bank_locator = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Type: ")) != NULL) { cur += 6; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_type, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_type = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Type Detail: ")) != NULL) { cur += 13; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_type_detail, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_type_detail = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Speed: ")) != NULL) { cur += 7; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_speed, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_speed = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Manufacturer: ")) != NULL) { cur += 14; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_manufacturer, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_manufacturer = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Serial Number: ")) != NULL) { cur += 15; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_serial_number, - cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_serial_number = g_strndup(cur, eol - cur); } if ((cur = strstr(base, "Part Number: ")) != NULL) { cur += 13; eol = strchr(cur, '\n'); virSkipSpacesBackwards(cur, &eol); - if (eol && VIR_STRNDUP(memory->memory_part_number, cur, eol - cur) < 0) - return -1; + if (eol) + memory->memory_part_number = g_strndup(cur, eol - cur); } next: -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Use g_strndup in all the cases where we check upfront whether a pointer is non-NULL and then use it to calculate the copied length.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virsysinfo.c | 241 ++++++++++++++++++++---------------------- 1 file changed, 112 insertions(+), 129 deletions(-)
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Remove the usage where sanity of the length argument is verified by other conditions not matching the previous patches. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libxl/xen_common.c | 18 ++++++++---------- tools/vsh.c | 5 ++--- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index a4a9ec59bf..7ac24fb606 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -817,9 +817,8 @@ xenParseSxprChar(const char *value, goto error; } - if (offset != value && - VIR_STRNDUP(def->source->data.tcp.host, value, offset - value) < 0) - goto error; + if (offset != value) + def->source->data.tcp.host = g_strndup(value, offset - value); offset2 = strchr(offset, ','); offset++; @@ -845,9 +844,9 @@ xenParseSxprChar(const char *value, goto error; } - if (offset != value && - VIR_STRNDUP(def->source->data.udp.connectHost, value, offset - value) < 0) - goto error; + if (offset != value) + def->source->data.udp.connectHost = g_strndup(value, + offset - value); offset2 = strchr(offset, '@'); if (offset2 != NULL) { @@ -862,10 +861,9 @@ xenParseSxprChar(const char *value, goto error; } - if (offset3 > (offset2 + 1) && - VIR_STRNDUP(def->source->data.udp.bindHost, - offset2 + 1, offset3 - offset2 - 1) < 0) - goto error; + if (offset3 > (offset2 + 1)) + def->source->data.udp.bindHost = g_strndup(offset2 + 1, + offset3 - offset2 - 1); def->source->data.udp.bindService = g_strdup(offset3 + 1); } else { diff --git a/tools/vsh.c b/tools/vsh.c index 000cf6a009..2b1e0506ba 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -360,9 +360,8 @@ vshCmddefCheckInternals(vshControl *ctl, cmd->name); return -1; /* alias options are tracked by the original name */ } - if ((p = strchr(name, '=')) && - VIR_STRNDUP(name, name, p - name) < 0) - vshErrorOOM(); + if ((p = strchr(name, '='))) + name = g_strndup(name, p - name); for (j = i + 1; cmd->opts[j].name; j++) { if (STREQ(name, cmd->opts[j].name) && cmd->opts[j].type != VSH_OT_ALIAS) -- 2.21.0

This patch failed to compile for me with the following error: ../../tools/vsh.c:70:1: error: 'vshErrorOOM' defined but not used [-Werror=unused-function] 70 | vshErrorOOM(void) | ^~~~~~~~~~~ Apparently you removed the last call to vshErrorOOM() in the tools/vsh.c changes here. For reference, I am testing this series on top of this commit: commit 54dd0938375daaa68674b271f444b312d6684b01 (origin/master, origin/HEAD, master) Author: Ján Tomko <jtomko@redhat.com> Date: Wed Nov 13 09:51:45 2019 +0100 locking: fix build with older sanlock Removing the function makes the patch compile without errors: diff --git a/tools/vsh.c b/tools/vsh.c index 2b1e0506ba..5005b1deaa 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -65,17 +65,6 @@ const vshCmdGrp *cmdGroups; const vshCmdDef *cmdSet; -/* simple handler for oom conditions */ -static void -vshErrorOOM(void) -{ - fflush(stdout); - fputs(_("error: Out of memory\n"), stderr); - fflush(stderr); - exit(EXIT_FAILURE); -} - - double vshPrettyCapacity(unsigned long long val, const char **unit) { Aside from this, patch LGTM. Thanks, DHB On 11/12/19 2:02 PM, Ján Tomko wrote:
Remove the usage where sanity of the length argument is verified by other conditions not matching the previous patches.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libxl/xen_common.c | 18 ++++++++---------- tools/vsh.c | 5 ++--- 2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index a4a9ec59bf..7ac24fb606 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -817,9 +817,8 @@ xenParseSxprChar(const char *value, goto error; }
- if (offset != value && - VIR_STRNDUP(def->source->data.tcp.host, value, offset - value) < 0) - goto error; + if (offset != value) + def->source->data.tcp.host = g_strndup(value, offset - value);
offset2 = strchr(offset, ','); offset++; @@ -845,9 +844,9 @@ xenParseSxprChar(const char *value, goto error; }
- if (offset != value && - VIR_STRNDUP(def->source->data.udp.connectHost, value, offset - value) < 0) - goto error; + if (offset != value) + def->source->data.udp.connectHost = g_strndup(value, + offset - value);
offset2 = strchr(offset, '@'); if (offset2 != NULL) { @@ -862,10 +861,9 @@ xenParseSxprChar(const char *value, goto error; }
- if (offset3 > (offset2 + 1) && - VIR_STRNDUP(def->source->data.udp.bindHost, - offset2 + 1, offset3 - offset2 - 1) < 0) - goto error; + if (offset3 > (offset2 + 1)) + def->source->data.udp.bindHost = g_strndup(offset2 + 1, + offset3 - offset2 - 1);
def->source->data.udp.bindService = g_strdup(offset3 + 1); } else { diff --git a/tools/vsh.c b/tools/vsh.c index 000cf6a009..2b1e0506ba 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -360,9 +360,8 @@ vshCmddefCheckInternals(vshControl *ctl, cmd->name); return -1; /* alias options are tracked by the original name */ } - if ((p = strchr(name, '=')) && - VIR_STRNDUP(name, name, p - name) < 0) - vshErrorOOM(); + if ((p = strchr(name, '='))) + name = g_strndup(name, p - name); for (j = i + 1; cmd->opts[j].name; j++) { if (STREQ(name, cmd->opts[j].name) && cmd->opts[j].type != VSH_OT_ALIAS)

Remove all the uses that use subtraction in their length argument. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/bhyve/bhyve_parse_command.c | 6 ++---- src/libxl/xen_common.c | 8 +++----- src/libxl/xen_xm.c | 3 +-- src/lxc/lxc_driver.c | 3 +-- src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_monitor_json.c | 8 ++------ src/util/vircgroupv1.c | 5 ++--- src/util/vircommand.c | 5 ++--- src/util/virconf.c | 24 +++++++----------------- src/util/virkeyfile.c | 6 ++---- src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 4 +--- src/util/virsysinfo.c | 3 +-- src/vmware/vmware_conf.c | 6 +----- 14 files changed, 27 insertions(+), 60 deletions(-) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 5c5dfae4a9..93c9c10b39 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -283,8 +283,7 @@ bhyveParseBhyveLPCArg(virDomainDefPtr def, if (!separator) goto error; - if (VIR_STRNDUP(type, arg, separator - arg) < 0) - goto error; + type = g_strndup(arg, separator - arg); /* Only support com%d */ if (STRPREFIX(type, "com") && type[4] == 0) { @@ -579,8 +578,7 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def, else separator++; /* Skip comma */ - if (VIR_STRNDUP(slotdef, arg, separator - arg - 1) < 0) - goto error; + slotdef = g_strndup(arg, separator - arg - 1); conf = strchr(separator+1, ','); if (conf) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 7ac24fb606..e6d022f253 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -850,9 +850,8 @@ xenParseSxprChar(const char *value, offset2 = strchr(offset, '@'); if (offset2 != NULL) { - if (VIR_STRNDUP(def->source->data.udp.connectService, - offset + 1, offset2 - offset - 1) < 0) - goto error; + def->source->data.udp.connectService = g_strndup(offset + 1, + offset2 - offset - 1); offset3 = strchr(offset2, ':'); if (offset3 == NULL) { @@ -994,8 +993,7 @@ xenParseVifBridge(virDomainNetDefPtr net, char *bridge) if ((vlanstr = strchr(bridge, '.'))) { /* 'bridge' string contains a bridge name and single vlan tag */ - if (VIR_STRNDUP(net->data.bridge.brname, bridge, vlanstr - bridge) < 0) - return -1; + net->data.bridge.brname = g_strndup(bridge, vlanstr - bridge); vlanstr++; if (virStrToLong_ui(vlanstr, NULL, 10, &tag) < 0) diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index 88cf9ac5b0..a0e649f4ef 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -131,8 +131,7 @@ xenParseXMDisk(char *entry, int hvm) /* No source file given, eg CDROM with no media */ ignore_value(virDomainDiskSetSource(disk, NULL)); } else { - if (VIR_STRNDUP(tmp, head, offset - head) < 0) - goto error; + tmp = g_strndup(head, offset - head); if (virDomainDiskSetSource(disk, tmp) < 0) { VIR_FREE(tmp); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index ea86451b09..ef3649800b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2150,8 +2150,7 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, if (!p) goto parse_error; - if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) - goto cleanup; + result[i].path = g_strndup(temp, p - temp); /* value */ temp = p + 1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f4ff2ba292..9e26af8af7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9289,8 +9289,7 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, if (!p) goto parse_error; - if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) - goto cleanup; + result[i].path = g_strndup(temp, p - temp); /* value */ temp = p + 1; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 10f6a4cadc..963ca58f43 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -644,15 +644,11 @@ qemuMonitorJSONParseKeywords(const char *str, separator = endmark; } - if (VIR_STRNDUP(keyword, start, separator - start) < 0) - goto error; + keyword = g_strndup(start, separator - start); if (separator < endmark) { separator++; - if (VIR_STRNDUP(value, separator, endmark - separator) < 0) { - VIR_FREE(keyword); - goto error; - } + value = g_strndup(separator, endmark - separator); if (strchr(value, ',')) { char *p = strchr(value, ',') + 1; char *q = p + 1; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index c1083b6d18..c0b6aa8204 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -769,9 +769,8 @@ virCgroupV1IdentifyRoot(virCgroupPtr group) return NULL; } - if (VIR_STRNDUP(ret, group->legacy[i].mountPoint, - tmp - group->legacy[i].mountPoint) < 0) - return NULL; + ret = g_strndup(group->legacy[i].mountPoint, + tmp - group->legacy[i].mountPoint); return ret; } diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 49432ddfcb..0993d3e168 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -3145,9 +3145,8 @@ virCommandRunRegex(virCommandPtr cmd, /* NB vars[0] is the full pattern, so we offset j by 1 */ for (j = 1; j <= nvars[i]; j++) { - if (VIR_STRNDUP(groups[ngroup++], p + vars[j].rm_so, - vars[j].rm_eo - vars[j].rm_so) < 0) - goto cleanup; + groups[ngroup++] = g_strndup(p + vars[j].rm_so, + vars[j].rm_eo - vars[j].rm_so); } } diff --git a/src/util/virconf.c b/src/util/virconf.c index cc88000387..4a16037284 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -386,8 +386,7 @@ virConfParseString(virConfParserCtxtPtr ctxt) virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated string")); return NULL; } - if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0) - return NULL; + ret = g_strndup(base, ctxt->cur - base); NEXT; } else if ((ctxt->cur + 6 < ctxt->end) && (STRPREFIX(ctxt->cur, "\"\"\""))) { @@ -407,8 +406,7 @@ virConfParseString(virConfParserCtxtPtr ctxt) virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated string")); return NULL; } - if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0) - return NULL; + ret = g_strndup(base, ctxt->cur - base); ctxt->cur += 3; } else if (CUR == '"') { NEXT; @@ -419,8 +417,7 @@ virConfParseString(virConfParserCtxtPtr ctxt) virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated string")); return NULL; } - if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0) - return NULL; + ret = g_strndup(base, ctxt->cur - base); NEXT; } else if (ctxt->conf->flags & VIR_CONF_FLAG_LXC_FORMAT) { base = ctxt->cur; @@ -430,8 +427,7 @@ virConfParseString(virConfParserCtxtPtr ctxt) /* Reverse to exclude the trailing blanks from the value */ while ((ctxt->cur > base) && (c_isblank(CUR))) ctxt->cur--; - if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0) - return NULL; + ret = g_strndup(base, ctxt->cur - base); } return ret; } @@ -567,8 +563,7 @@ virConfParseName(virConfParserCtxtPtr ctxt) ((ctxt->conf->flags & VIR_CONF_FLAG_LXC_FORMAT) && (CUR == '.')))) NEXT; - if (VIR_STRNDUP(ret, base, ctxt->cur - base) < 0) - return NULL; + ret = g_strndup(base, ctxt->cur - base); return ret; } @@ -591,8 +586,7 @@ virConfParseComment(virConfParserCtxtPtr ctxt) NEXT; base = ctxt->cur; while ((ctxt->cur < ctxt->end) && (!IS_EOL(CUR))) NEXT; - if (VIR_STRNDUP(comm, base, ctxt->cur - base) < 0) - return -1; + comm = g_strndup(base, ctxt->cur - base); if (virConfAddEntry(ctxt->conf, NULL, NULL, comm) == NULL) { VIR_FREE(comm); return -1; @@ -666,11 +660,7 @@ virConfParseStatement(virConfParserCtxtPtr ctxt) NEXT; base = ctxt->cur; while ((ctxt->cur < ctxt->end) && (!IS_EOL(CUR))) NEXT; - if (VIR_STRNDUP(comm, base, ctxt->cur - base) < 0) { - VIR_FREE(name); - virConfFreeValue(value); - return -1; - } + comm = g_strndup(base, ctxt->cur - base); } if (virConfAddEntry(ctxt->conf, name, value, comm) == NULL) { VIR_FREE(name); diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index c7e756d26a..c5af85e72b 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -116,8 +116,7 @@ static int virKeyFileParseGroup(virKeyFileParserCtxtPtr ctxt) return -1; } - if (VIR_STRNDUP(ctxt->groupname, name, ctxt->cur - name) < 0) - return -1; + ctxt->groupname = g_strndup(name, ctxt->cur - name); NEXT; @@ -160,8 +159,7 @@ static int virKeyFileParseValue(virKeyFileParserCtxtPtr ctxt) return -1; } - if (VIR_STRNDUP(key, keystart, ctxt->cur - keystart) < 0) - return -1; + key = g_strndup(keystart, ctxt->cur - keystart); NEXT; valuestart = ctxt->cur; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 77f885ef1d..a6ef64f73d 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2946,8 +2946,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, return -1; } - if (VIR_STRNDUP(protocol, path, p - path) < 0) - return -1; + protocol = g_strndup(path, p - path); if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virstring.c b/src/util/virstring.c index 283cf8c8d8..1b0b5dae97 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1058,9 +1058,7 @@ virStringSearch(const char *str, if (VIR_EXPAND_N(*matches, nmatches, 1) < 0) goto cleanup; - if (VIR_STRNDUP(match, str + rem.rm_so, - rem.rm_eo - rem.rm_so) < 0) - goto cleanup; + match = g_strndup(str + rem.rm_so, rem.rm_eo - rem.rm_so); VIR_DEBUG("Got '%s'", match); diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 635e4c4bfb..fe43dfdd75 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -485,8 +485,7 @@ virSysinfoParseS390Delimited(const char *base, const char *name, char **value, start += 1; end = strchrnul(start, delim2); virSkipSpaces(&start); - if (VIR_STRNDUP(*value, start, end - start) < 0) - return NULL; + *value = g_strndup(start, end - start); virTrimSpaces(*value, NULL); return end; } diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index ef7696a496..6b2c8888ab 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -348,8 +348,7 @@ vmwareParsePath(const char *path, char **directory, char **filename) return -1; } - if (VIR_STRNDUP(*directory, path, separator - path - 1) < 0) - goto error; + *directory = g_strndup(path, separator - path - 1); *filename = g_strdup(separator); } else { @@ -357,9 +356,6 @@ vmwareParsePath(const char *path, char **directory, char **filename) } return 0; - - error: - return -1; } void -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Remove all the uses that use subtraction in their length argument.
Signed-off-by: Ján Tomko <jtomko@redhat.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Replace all the uses passing a single parameter as the length. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/nwfilter_conf.c | 4 ++-- src/conf/nwfilter_params.c | 6 ++---- src/interface/interface_backend_udev.c | 8 ++------ src/libxl/xen_common.c | 3 +-- src/libxl/xen_xl.c | 6 ++---- src/libxl/xen_xm.c | 6 ++---- src/qemu/qemu_monitor_json.c | 3 +-- src/rpc/virnetlibsshsession.c | 7 ++----- src/storage/storage_backend_logical.c | 7 ++----- src/util/viriscsi.c | 3 +-- src/util/virjson.c | 11 +++-------- src/util/virkeyfile.c | 3 +-- src/util/virsocketaddr.c | 5 +---- src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 3 +-- 15 files changed, 24 insertions(+), 54 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 6c7b85d4c8..7a4be5d4ec 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -401,8 +401,8 @@ virNWFilterRuleDefAddString(virNWFilterRuleDefPtr nwf, { char *tmp; - if (VIR_STRNDUP(tmp, string, maxstrlen) < 0 || - VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0) + tmp = g_strndup(string, maxstrlen); + if (VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0) VIR_FREE(tmp); return tmp; diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 7608d4960e..4549b7a71c 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -884,8 +884,7 @@ virNWFilterVarAccessParse(const char *varAccess) if (input[idx] == '\0') { /* in the form 'IP', which is equivalent to IP[@0] */ - if (VIR_STRNDUP(dest->varName, input, idx) < 0) - goto err_exit; + dest->varName = g_strndup(input, idx); dest->accessType = VIR_NWFILTER_VAR_ACCESS_ITERATOR; dest->u.iterId = 0; return dest; @@ -898,8 +897,7 @@ virNWFilterVarAccessParse(const char *varAccess) varNameLen = idx; - if (VIR_STRNDUP(dest->varName, input, varNameLen) < 0) - goto err_exit; + dest->varName = g_strndup(input, varNameLen); input += idx + 1; virSkipSpaces(&input); diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index b7b06ed67a..531f9e90e5 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -954,12 +954,8 @@ udevGetIfaceDefVlan(struct udev *udev G_GNUC_UNUSED, goto cleanup; } - if (VIR_STRNDUP(ifacedef->data.vlan.tag, vid_pos, vid_len) < 0) - goto cleanup; - if (VIR_STRNDUP(ifacedef->data.vlan.dev_name, dev_pos, dev_len) < 0) { - VIR_FREE(ifacedef->data.vlan.tag); - goto cleanup; - } + ifacedef->data.vlan.tag = g_strndup(vid_pos, vid_len); + ifacedef->data.vlan.dev_name = g_strndup(dev_pos, dev_len); ret = 0; diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index e6d022f253..66e7850245 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1164,8 +1164,7 @@ xenParseVif(char *entry, const char *vif_typename) } else if (STRPREFIX(key, "script=")) { int len = nextkey ? (nextkey - data) : strlen(data); VIR_FREE(script); - if (VIR_STRNDUP(script, data, len) < 0) - return NULL; + script = g_strndup(data, len); } else if (STRPREFIX(key, "model=")) { int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(model, data, len, sizeof(model)) < 0) { diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 396adf6dac..4e2bdb2ed0 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1109,13 +1109,11 @@ xenParseXLChannel(virConfPtr conf, virDomainDefPtr def) } else if (STRPREFIX(key, "name=")) { int len = nextkey ? (nextkey - data) : strlen(data); VIR_FREE(name); - if (VIR_STRNDUP(name, data, len) < 0) - goto cleanup; + name = g_strndup(data, len); } else if (STRPREFIX(key, "path=")) { int len = nextkey ? (nextkey - data) : strlen(data); VIR_FREE(path); - if (VIR_STRNDUP(path, data, len) < 0) - goto cleanup; + path = g_strndup(data, len); } while (nextkey && (nextkey[0] == ',' || diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c index a0e649f4ef..7dea2998f2 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -167,8 +167,7 @@ xenParseXMDisk(char *entry, int hvm) /* The main type phy:, file:, tap: ... */ if ((tmp = strchr(src, ':')) != NULL) { len = tmp - src; - if (VIR_STRNDUP(tmp, src, len) < 0) - goto error; + tmp = g_strndup(src, len); if (virDomainDiskSetDriver(disk, tmp) < 0) { VIR_FREE(tmp); @@ -192,8 +191,7 @@ xenParseXMDisk(char *entry, int hvm) goto error; len = tmp - src; - if (VIR_STRNDUP(driverType, src, len) < 0) - goto error; + driverType = g_strndup(src, len); if (STREQ(driverType, "aio")) virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 963ca58f43..aa2598a85d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -272,8 +272,7 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon, if (nl) { int got = nl - (data + used); char *line; - if (VIR_STRNDUP(line, data + used, got) < 0) - return -1; + line = g_strndup(data + used, got); used += got + strlen(LINE_ENDING); line[got] = '\0'; /* kill \n */ if (qemuMonitorJSONIOProcessLine(mon, line, msg) < 0) { diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index 2312939cdc..e874929740 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -431,9 +431,7 @@ virNetLibsshAuthenticatePrivkeyCb(const char *prompt, goto error; } - if (VIR_STRNDUP(actual_prompt, prompt, - virLengthForPromptString(prompt)) < 0) - goto error; + actual_prompt = g_strndup(prompt, virLengthForPromptString(prompt)); memset(&retr_passphrase, 0, sizeof(virConnectCredential)); retr_passphrase.type = cred_type; @@ -716,8 +714,7 @@ virNetLibsshAuthenticateKeyboardInteractive(virNetLibsshSessionPtr sess, prompt = virBufferContentAndReset(&prompt_buff); } else { - if (VIR_STRNDUP(prompt, promptStr, promptStrLen) < 0) - goto prompt_error; + prompt = g_strndup(promptStr, promptStrLen); } memset(&retr_passphrase, 0, sizeof(virConnectCredential)); diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 48023abd1a..ba3c5ea70e 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -214,13 +214,10 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, len = vars[j].rm_eo - vars[j].rm_so; p[vars[j].rm_eo] = '\0'; - if (VIR_STRNDUP(extent.path, - p + vars[j].rm_so, len) < 0) - goto cleanup; + extent.path = g_strndup(p + vars[j].rm_so, len); len = vars[j + 1].rm_eo - vars[j + 1].rm_so; - if (VIR_STRNDUP(offset_str, p + vars[j + 1].rm_so, len) < 0) - goto cleanup; + offset_str = g_strndup(p + vars[j + 1].rm_so, len); if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 9f4c8f4e03..84fa542b7c 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -162,8 +162,7 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (!(next = strchr(current, ' '))) goto error; - if (VIR_STRNDUP(iface, current, (next - current)) < 0) - goto cleanup; + iface = g_strndup(current, (next - current)); current = next + 1; diff --git a/src/util/virjson.c b/src/util/virjson.c index 76efb17e24..184d082dc5 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -463,10 +463,7 @@ virJSONValueNewStringLen(const char *data, return NULL; val->type = VIR_JSON_TYPE_STRING; - if (VIR_STRNDUP(val->data.string, data, length) < 0) { - VIR_FREE(val); - return NULL; - } + val->data.string = g_strndup(data, length); return val; } @@ -1633,8 +1630,7 @@ virJSONParserHandleNumber(void *ctx, char *str; virJSONValuePtr value; - if (VIR_STRNDUP(str, s, l) < 0) - return -1; + str = g_strndup(s, l); value = virJSONValueNewNumber(str); VIR_FREE(str); @@ -1691,8 +1687,7 @@ virJSONParserHandleMapKey(void *ctx, state = &parser->state[parser->nstate-1]; if (state->key) return 0; - if (VIR_STRNDUP(state->key, (const char *)stringVal, stringLen) < 0) - return 0; + state->key = g_strndup((const char *)stringVal, stringLen); return 1; } diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index c5af85e72b..998ce9caf7 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -172,8 +172,7 @@ static int virKeyFileParseValue(virKeyFileParserCtxtPtr ctxt) len = ctxt->cur - valuestart; if (IS_EOF && !IS_EOL(CUR)) len++; - if (VIR_STRNDUP(value, valuestart, len) < 0) - goto cleanup; + value = g_strndup(valuestart, len); if (virHashAddEntry(ctxt->group, key, value) < 0) { VIR_FREE(value); diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index d7b0f12d96..ef51d3fa7d 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -593,10 +593,7 @@ virSocketAddrGetPath(virSocketAddrPtr addr G_GNUC_UNUSED) return NULL; } - if (VIR_STRNDUP(path, - addr->data.un.sun_path, - sizeof(addr->data.un.sun_path)) < 0) - return NULL; + path = g_strndup(addr->data.un.sun_path, sizeof(addr->data.un.sun_path)); return path; #else diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a6ef64f73d..8c99c349a6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -421,8 +421,7 @@ cowGetBackingStore(char **res, return BACKING_STORE_OK; } - if (VIR_STRNDUP(*res, (const char*)buf + 4 + 4, COW_FILENAME_MAXLEN) < 0) - return BACKING_STORE_ERROR; + *res = g_strndup((const char *)buf + 4 + 4, COW_FILENAME_MAXLEN); return BACKING_STORE_OK; } diff --git a/src/util/virstring.c b/src/util/virstring.c index 1b0b5dae97..cd4418d144 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -91,8 +91,7 @@ virStringSplitCount(const char *string, if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0) goto error; - if (VIR_STRNDUP(tokens[ntokens], remainder, len) < 0) - goto error; + tokens[ntokens] = g_strndup(remainder, len); ntokens++; remainder = tmp + delimlen; tmp = strstr(remainder, delim); -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Replace all the uses passing a single parameter as the length.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/nwfilter_conf.c | 4 ++-- src/conf/nwfilter_params.c | 6 ++---- src/interface/interface_backend_udev.c | 8 ++------ src/libxl/xen_common.c | 3 +-- src/libxl/xen_xl.c | 6 ++---- src/libxl/xen_xm.c | 6 ++---- src/qemu/qemu_monitor_json.c | 3 +-- src/rpc/virnetlibsshsession.c | 7 ++----- src/storage/storage_backend_logical.c | 7 ++----- src/util/viriscsi.c | 3 +-- src/util/virjson.c | 11 +++-------- src/util/virkeyfile.c | 3 +-- src/util/virsocketaddr.c | 5 +---- src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 3 +-- 15 files changed, 24 insertions(+), 54 deletions(-)
[...]
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 9f4c8f4e03..84fa542b7c 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -162,8 +162,7 @@ virStorageBackendIQNFound(const char *initiatoriqn, if (!(next = strchr(current, ' '))) goto error;
- if (VIR_STRNDUP(iface, current, (next - current)) < 0) - goto cleanup; + iface = g_strndup(current, (next - current));
current = next + 1;
We can live without the parenthesis around 'next - current'. Also, I think this change belongs to the previous patch that handled subtraction in the length argument of VIR_STRNDUP(). Thanks, DHB

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virstringtest.c | 136 ------------------------------------------ 1 file changed, 136 deletions(-) diff --git a/tests/virstringtest.c b/tests/virstringtest.c index 1e408f2757..992b25df92 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -236,136 +236,6 @@ static int testRemove(const void *args) } -static bool fail; - -static const char * -testStrdupLookup1(size_t i) -{ - switch (i) { - case 0: - return "hello"; - case 1: - return NULL; - default: - fail = true; - return "oops"; - } -} - -static size_t -testStrdupLookup2(size_t i) -{ - if (i) - fail = true; - return 5; -} - -static int -testStrdup(const void *data G_GNUC_UNUSED) -{ - char *array[] = { NULL, NULL }; - size_t i = 0; - size_t j = 0; - size_t k = 0; - int ret = -1; - int value; - - value = VIR_STRDUP(array[i++], testStrdupLookup1(j++)); - if (value != 1) { - virFilePrintf(stderr, "unexpected strdup result %d, expected 1\n", value); - goto cleanup; - } - /* coverity[dead_error_begin] */ - if (i != 1) { - virFilePrintf(stderr, "unexpected side effects i=%zu, expected 1\n", i); - goto cleanup; - } - /* coverity[dead_error_begin] */ - if (j != 1) { - virFilePrintf(stderr, "unexpected side effects j=%zu, expected 1\n", j); - goto cleanup; - } - if (STRNEQ_NULLABLE(array[0], "hello") || array[1]) { - virFilePrintf(stderr, "incorrect array contents '%s' '%s'\n", - NULLSTR(array[0]), NULLSTR(array[1])); - goto cleanup; - } - - value = VIR_STRNDUP(array[i++], testStrdupLookup1(j++), - testStrdupLookup2(k++)); - if (value != 0) { - virFilePrintf(stderr, "unexpected strdup result %d, expected 0\n", value); - goto cleanup; - } - /* coverity[dead_error_begin] */ - if (i != 2) { - virFilePrintf(stderr, "unexpected side effects i=%zu, expected 2\n", i); - goto cleanup; - } - /* coverity[dead_error_begin] */ - if (j != 2) { - virFilePrintf(stderr, "unexpected side effects j=%zu, expected 2\n", j); - goto cleanup; - } - /* coverity[dead_error_begin] */ - if (k != 1) { - virFilePrintf(stderr, "unexpected side effects k=%zu, expected 1\n", k); - goto cleanup; - } - if (STRNEQ_NULLABLE(array[0], "hello") || array[1]) { - virFilePrintf(stderr, "incorrect array contents '%s' '%s'\n", - NULLSTR(array[0]), NULLSTR(array[1])); - goto cleanup; - } - - if (fail) { - virFilePrintf(stderr, "side effects failed\n"); - goto cleanup; - } - - ret = 0; - cleanup: - for (i = 0; i < G_N_ELEMENTS(array); i++) - VIR_FREE(array[i]); - return ret; -} - -static int -testStrndupNegative(const void *opaque G_GNUC_UNUSED) -{ - int ret = -1; - char *dst; - const char *src = "Hello world"; - int value; - - if ((value = VIR_STRNDUP(dst, src, 5)) != 1) { - fprintf(stderr, "unexpected virStrndup result %d, expected 1\n", value); - goto cleanup; - } - - if (STRNEQ_NULLABLE(dst, "Hello")) { - fprintf(stderr, "unexpected content '%s'", dst); - goto cleanup; - } - - VIR_FREE(dst); - if ((value = VIR_STRNDUP(dst, src, -1)) != 1) { - fprintf(stderr, "unexpected virStrndup result %d, expected 1\n", value); - goto cleanup; - } - - if (STRNEQ_NULLABLE(dst, src)) { - fprintf(stderr, "unexpected content '%s'", dst); - goto cleanup; - } - - ret = 0; - cleanup: - VIR_FREE(dst); - return ret; -} - - static int testStringSortCompare(const void *opaque G_GNUC_UNUSED) { @@ -853,12 +723,6 @@ mymain(void) const char *tokens8[] = { "gluster", "rdma", NULL }; TEST_SPLIT("gluster+rdma", "+", 2, tokens8); - if (virTestRun("strdup", testStrdup, NULL) < 0) - ret = -1; - - if (virTestRun("strdup", testStrndupNegative, NULL) < 0) - ret = -1; - if (virTestRun("virStringSortCompare", testStringSortCompare, NULL) < 0) ret = -1; -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virstringtest.c | 136 ------------------------------------------ 1 file changed, 136 deletions(-)
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 2 -- src/util/virstring.c | 49 ---------------------------- src/util/virstring.h | 69 ---------------------------------------- 3 files changed, 120 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5787338e92..00e5054689 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3073,7 +3073,6 @@ virSkipSpaces; virSkipSpacesAndBackslash; virSkipSpacesBackwards; virStrcpy; -virStrdup; virStringBufferIsPrintable; virStringFilterChars; virStringHasCaseSuffix; @@ -3108,7 +3107,6 @@ virStringStripSuffix; virStringToUpper; virStringTrimOptionalNewline; virStrncpy; -virStrndup; virStrToDouble; virStrToLong_i; virStrToLong_l; diff --git a/src/util/virstring.c b/src/util/virstring.c index cd4418d144..10e544f959 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -888,55 +888,6 @@ virStringIsEmpty(const char *str) return str[0] == '\0'; } -/** - * virStrdup: - * @dest: where to store duplicated string - * @src: the source string to duplicate - * - * Wrapper over strdup, which aborts on OOM error. - * - * Returns: 0 for NULL src, 1 on successful copy, aborts on OOM - */ -int -virStrdup(char **dest, - const char *src) -{ - *dest = NULL; - if (!src) - return 0; - *dest = g_strdup(src); - - return 1; -} - -/** - * virStrndup: - * @dest: where to store duplicated string - * @src: the source string to duplicate - * @n: how many bytes to copy - * - * Wrapper over strndup, which aborts on OOM error. - * - * In case @n is smaller than zero, the whole @src string is - * copied. - * - * Returns: 0 for NULL src, 1 on successful copy, aborts on OOM - */ -int -virStrndup(char **dest, - const char *src, - ssize_t n) -{ - *dest = NULL; - if (!src) - return 0; - if (n < 0) - n = strlen(src); - *dest = g_strndup(src, n); - - return 1; -} - size_t virStringListLength(const char * const *strings) { diff --git a/src/util/virstring.h b/src/util/virstring.h index 081a5ff1aa..a2cd92cf83 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -134,75 +134,6 @@ int virStrdup(char **dest, const char *src) int virStrndup(char **dest, const char *src, ssize_t n) G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1); -/** - * VIR_STRDUP: - * @dst: variable to hold result (char*, not char**) - * @src: string to duplicate - * - * DEPRECATED: use g_strdup instead - * - * Duplicate @src string and store it into @dst. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM - */ -#define VIR_STRDUP(dst, src) virStrdup(&(dst), src) - -/** - * VIR_STRDUP_QUIET: - * @dst: variable to hold result (char*, not char**) - * @src: string to duplicate - * - * DEPRECATED: use g_strdup instead - * - * Duplicate @src string and store it into @dst. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM - */ -#define VIR_STRDUP_QUIET(dst, src) VIR_STRDUP(dst, src) - -/** - * VIR_STRNDUP: - * @dst: variable to hold result (char*, not char**) - * @src: string to duplicate - * @n: the maximum number of bytes to copy - * - * DEPRECATED: use g_strndup instead - * - * Duplicate @src string and store it into @dst. If @src is longer than @n, - * only @n bytes are copied and terminating null byte '\0' is added. If @n - * is a negative number, then the whole @src string is copied. That is, - * VIR_STRDUP(dst, src) and VIR_STRNDUP(dst, src, -1) are equal. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM - */ -#define VIR_STRNDUP(dst, src, n) virStrndup(&(dst), src, n) - -/** - * VIR_STRNDUP_QUIET: - * @dst: variable to hold result (char*, not char**) - * @src: string to duplicate - * @n: the maximum number of bytes to copy - * - * DEPRECATED: use g_strndup instead - * - * Duplicate @src string and store it into @dst. If @src is longer than @n, - * only @n bytes are copied and terminating null byte '\0' is added. If @n - * is a negative number, then the whole @src string is copied. That is, - * VIR_STRDUP_QUIET(dst, src) and VIR_STRNDUP_QUIET(dst, src, -1) are - * equal. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM - */ -#define VIR_STRNDUP_QUIET(dst, src, n) virStrndup(&(dst), src, n) - size_t virStringListLength(const char * const *strings); int virStringSortCompare(const void *a, const void *b); -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 2 -- src/util/virstring.c | 49 ---------------------------- src/util/virstring.h | 69 ---------------------------------------- 3 files changed, 120 deletions(-)
(assuming the VIR_STRNDUP introduced by Jirka's commit 0861080f0e23921 is taken care of) Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/hacking.html.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 06deab9e90..74aba5d46b 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1040,9 +1040,6 @@ BAD: a single method. Keep the style consistent, converting existing code to GLib style in a separate, prior commit.</dd> - <dt><code>VIR_STRDUP</code>, <code>VIR_STRNDUP</code></dt> - <dd>Prefer the GLib APIs <code>g_strdup</code> and <code>g_strndup</code>.</dd> - <dt><code>virStrerror</code></dt> <dd>The GLib <code>g_strerror()</code> function should be used instead, which has a simpler calling convention as an added benefit.</dd> @@ -1071,8 +1068,6 @@ BAD: <p>String allocation macros and functions:</p> <table class="top_table"> <tr><th>deprecated version</th><th>GLib version</th><th>Notes</th></tr> - <tr><td><code>VIR_STRDUP</code></td><td><code>g_strdup</code></td><td></td></tr> - <tr><td><code>VIR_STRNDUP</code></td><td><code>g_strndup</code></td><td></td></tr> <tr><td><code>virAsprintf</code></td><td><code>g_strdup_printf</code></td><td></td></tr> <tr><td><code>virVasprintf</code></td><td><code>g_strdup_vprint</code></td> <td>use <code>g_vasprintf</code> if you really need to know the returned length</td></tr> @@ -1108,6 +1103,9 @@ BAD: <dd>The GLib macros <code>g_autoptr</code> and <code>G_DEFINE_AUTOPTR_CLEANUP_FUNC</code> should be used to manage autoclean of virObject classes. This matches usage with GObject classes.</dd> + + <dt><code>VIR_STRDUP</code>, <code>VIR_STRNDUP</code></dt> + <dd>Prefer the GLib APIs <code>g_strdup</code> and <code>g_strndup</code>.</dd> </dl> <table class="top_table"> <tr><th>deleted version</th><th>GLib version</th><th>Notes</th></tr> @@ -1128,6 +1126,8 @@ BAD: <tr><td><code>ATTRIBUTE_RETURN_CHECK</code></td><td><code>G_GNUC_WARN_UNUSED_RESULT</code></td><td></td></tr> <tr><td><code>ATTRIBUTE_SENTINEL</code></td><td><code>G_GNUC_NULL_TERMINATED</code></td><td></td></tr> <tr><td><code>ATTRIBUTE_UNUSED</code></td><td><code>G_GNUC_UNUSED</code></td><td></td></tr> + <tr><td><code>VIR_STRDUP</code></td><td><code>g_strdup</code></td><td></td></tr> + <tr><td><code>VIR_STRNDUP</code></td><td><code>g_strndup</code></td><td></td></tr> </table> -- 2.21.0

On 11/12/19 2:02 PM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/hacking.html.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
participants (2)
-
Daniel Henrique Barboza
-
Ján Tomko