[libvirt] [PATCHv2 0/8] Remove VIR_STRNDUP
Ján Tomko (8): Remove VIR_STRDUP usage that sneaked in in the meantime vsh: remove vshErrorOOM Remove VIR_STRNDUP usage with checked pointers Remove all the uses that use subtraction in their length argument 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 | 6 +- src/conf/backup_conf.c | 4 +- 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 | 29 +++--- src/libxl/xen_xl.c | 6 +- src/libxl/xen_xm.c | 9 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_monitor_json.c | 14 +-- src/rpc/virnetlibsshsession.c | 7 +- src/util/vircgroupv1.c | 5 +- src/util/vircommand.c | 1 - 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 | 52 +--------- src/util/virstring.h | 69 ------------- src/util/virsysinfo.c | 3 +- src/vmware/vmware_conf.c | 6 +- tests/virstringtest.c | 136 ------------------------- tools/vsh.c | 16 +-- 28 files changed, 63 insertions(+), 394 deletions(-) -- 2.21.0
My hesitation to remove VIR_STRDUP without VIR_STRNDUP resulted in these being able to sneak in. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/backup_conf.c | 4 +--- src/qemu/qemu_monitor_json.c | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 6be2e0498e..aa11967d2a 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -483,9 +483,7 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def, continue; backupdisk = &def->disks[ndisks++]; - - if (VIR_STRDUP(backupdisk->name, domdisk->dst) < 0) - return -1; + backupdisk->name = g_strdup(domdisk->dst); if (backup_all && !virStorageSourceIsEmpty(domdisk->src) && diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 00e1d3ce15..8a8347924a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5564,8 +5564,7 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, goto cleanup; } - if (VIR_STRDUP(info->defaultCPU, tmp) < 0) - goto cleanup; + info->defaultCPU = g_strdup(tmp); } } -- 2.21.0
We abort on allocation errors now so there is no need to have a function for it. Replace the only use by return -1, chosen by fair dice roll. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/vsh.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 6c78a7a373..dd2c039b47 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -64,17 +64,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) { @@ -361,7 +350,7 @@ vshCmddefCheckInternals(vshControl *ctl, } if ((p = strchr(name, '=')) && VIR_STRNDUP(name, name, p - name) < 0) - vshErrorOOM(); + return -1; 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
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 8c07df80b6..912dd8834a 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -815,9 +815,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++; @@ -843,9 +842,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) { @@ -860,10 +859,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 dd2c039b47..5ccda5ab44 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -348,9 +348,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) - return -1; + 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
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.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 | 1 - src/util/virconf.c | 24 +++++++----------------- src/util/viriscsi.c | 3 +-- src/util/virkeyfile.c | 6 ++---- src/util/virstoragefile.c | 3 +-- src/util/virsysinfo.c | 3 +-- src/vmware/vmware_conf.c | 6 +----- 14 files changed, 25 insertions(+), 57 deletions(-) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 5eec05c7d4..e74c4c2d0c 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -282,8 +282,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) { @@ -578,8 +577,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 912dd8834a..89ced6ae2b 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -848,9 +848,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) { @@ -992,8 +991,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 1b462d0487..a196912194 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 b40a96b4ce..fbf49ee957 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2137,8 +2137,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 4d77fd6f6a..47665f4ea6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9251,8 +9251,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 8a8347924a..b2f695a70c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -643,15 +643,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 40eafd7552..4f6b89cf2e 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -780,9 +780,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 e677b67b93..dd37b0b8dc 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -3138,7 +3138,6 @@ virCommandRunRegex(virCommandPtr cmd, /* NB match #0 is the full pattern, so we offset j by 1 */ for (j = 1; j <= nvars[i]; j++) groups[ngroup++] = g_match_info_fetch(info, j); - } /* We've matched on the last regex, so callback time */ if (i == nregex) { diff --git a/src/util/virconf.c b/src/util/virconf.c index 3b45436499..dce84cabb7 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) && (IS_BLANK(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/viriscsi.c b/src/util/viriscsi.c index da27894b0b..ac48527dde 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -160,8 +160,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/virkeyfile.c b/src/util/virkeyfile.c index a98d60cdb1..ece31667ce 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -117,8 +117,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; @@ -161,8 +160,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 5e371694d1..8918f905a2 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2945,8 +2945,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/virsysinfo.c b/src/util/virsysinfo.c index ebb329bb6f..8132ab7a07 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 9f7d874c12..290e0e898d 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
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/util/virjson.c | 11 +++-------- src/util/virkeyfile.c | 3 +-- src/util/virsocketaddr.c | 5 +---- src/util/virstoragefile.c | 3 +-- src/util/virstring.c | 3 +-- 13 files changed, 21 insertions(+), 47 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 5bcf52a84c..d071c398bc 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -400,8 +400,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 b1a2c50f27..4110169135 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 7df5cf2cc3..26b1045f8a 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -953,12 +953,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 89ced6ae2b..415549a42c 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1153,8 +1153,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 cdff71f11b..8112214b5f 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1104,13 +1104,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 a196912194..5d5d521c18 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 b2f695a70c..b4d103a37b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -271,8 +271,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/util/virjson.c b/src/util/virjson.c index c57dc11f78..988a09e956 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -462,10 +462,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; } @@ -1632,8 +1629,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); @@ -1690,8 +1686,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 ece31667ce..84b1b0e55a 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -173,8 +173,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 8918f905a2..14e1efc9e8 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -420,8 +420,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 bd7c640042..7cf32fda3e 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -89,8 +89,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
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- tests/virstringtest.c | 136 ------------------------------------------ 1 file changed, 136 deletions(-) diff --git a/tests/virstringtest.c b/tests/virstringtest.c index fc5f9bf937..1f195ab4b9 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -233,136 +233,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) { @@ -847,12 +717,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
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.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 4a342239e5..38e2ab915b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3114,7 +3114,6 @@ virSkipSpaces; virSkipSpacesAndBackslash; virSkipSpacesBackwards; virStrcpy; -virStrdup; virStringBufferIsPrintable; virStringFilterChars; virStringHasCaseSuffix; @@ -3149,7 +3148,6 @@ virStringStripSuffix; virStringToUpper; virStringTrimOptionalNewline; virStrncpy; -virStrndup; virStrToDouble; virStrToLong_i; virStrToLong_l; diff --git a/src/util/virstring.c b/src/util/virstring.c index 7cf32fda3e..fe5c026d2c 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -886,55 +886,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
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.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 12/11/19 2:23 PM, Ján Tomko wrote:
Ján Tomko (8): Remove VIR_STRDUP usage that sneaked in in the meantime vsh: remove vshErrorOOM Remove VIR_STRNDUP usage with checked pointers Remove all the uses that use subtraction in their length argument 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 | 6 +- src/conf/backup_conf.c | 4 +- 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 | 29 +++--- src/libxl/xen_xl.c | 6 +- src/libxl/xen_xm.c | 9 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_monitor_json.c | 14 +-- src/rpc/virnetlibsshsession.c | 7 +- src/util/vircgroupv1.c | 5 +- src/util/vircommand.c | 1 - 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 | 52 +--------- src/util/virstring.h | 69 ------------- src/util/virsysinfo.c | 3 +- src/vmware/vmware_conf.c | 6 +- tests/virstringtest.c | 136 ------------------------- tools/vsh.c | 16 +-- 28 files changed, 63 insertions(+), 394 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko -
Michal Privoznik