[libvirt] [PATCH v2 0/8] virStr*cpy*() related fixes and cleanups
Changes from [v1]: * catch a bunch more suboptimal uses of the original API; * improve the API so that it's both more sensible and fits better with the rest of libvirt. [v1] https://www.redhat.com/archives/libvir-list/2018-July/msg01044.html Andrea Bolognani (8): src: Use virStrcpyStatic() to avoid truncation src: Use virStrcpyStatic() wherever possible src: Use VIR_STRDUP() wherever possible src: Use virStrcpy() wherever possible src: Don't rely on strncpy()-like behavior src: Make virStr*cpy*() functions return an int esx: Use memcpy() in esxVI_CURL_Debug() util: Improve virStrncpy() implementation docs/hacking.html.in | 29 +++++----- src/conf/capabilities.c | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/nwfilter_conf.c | 3 +- src/esx/esx_driver.c | 8 +-- src/esx/esx_vi.c | 6 +- src/esx/esx_vi_types.c | 2 +- src/hyperv/hyperv_driver.c | 3 +- src/libxl/libxl_conf.c | 2 +- src/locking/lock_driver_sanlock.c | 25 ++++---- src/lxc/lxc_driver.c | 8 +-- src/nwfilter/nwfilter_dhcpsnoop.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/openvz/openvz_conf.c | 8 +-- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/remote/remote_daemon_dispatch.c | 4 +- src/remote/remote_driver.c | 4 +- src/rpc/virnetlibsshsession.c | 4 +- src/rpc/virnetsocket.c | 4 +- src/security/security_apparmor.c | 2 +- src/security/virt-aa-helper.c | 2 +- src/test/test_driver.c | 4 +- src/uml/uml_driver.c | 4 +- src/util/virfdstream.c | 4 +- src/util/virhostcpu.c | 4 +- src/util/virhostmem.c | 6 +- src/util/virlog.c | 5 +- src/util/virnetdev.c | 12 ++-- src/util/virnetdevbridge.c | 6 +- src/util/virnetdevtap.c | 4 +- src/util/virnetdevvportprofile.c | 6 +- src/util/virstring.c | 70 +++++++++++++++-------- src/util/virstring.h | 4 +- src/util/virtypedparam.c | 8 +-- src/xenapi/xenapi_driver.c | 4 +- src/xenconfig/xen_common.c | 38 ++++++------ src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 41 ++++++------- src/xenconfig/xen_xm.c | 2 +- 42 files changed, 179 insertions(+), 179 deletions(-) -- 2.17.1
The way virStrncpy() is called here will never result in buffer overflow, but it won't prevent or detect truncation either, despite what the error message might suggest. Use virStrcpyStatic(), which does all of the above, instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/esx/esx_driver.c | 4 +--- src/hyperv/hyperv_driver.c | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 947b7c1a31..edd21b9d28 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1317,9 +1317,7 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - if (!virStrncpy(nodeinfo->model, dynamicProperty->val->string, - sizeof(nodeinfo->model) - 1, - sizeof(nodeinfo->model))) { + if (!virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU Model %s too long for destination"), dynamicProperty->val->string); diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index a85943668c..6f74adf372 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -307,8 +307,7 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) } /* Fill struct */ - if (virStrncpy(info->model, processorList->data.common->Name, - sizeof(info->model) - 1, sizeof(info->model)) == NULL) { + if (virStrcpyStatic(info->model, processorList->data.common->Name) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU model %s too long for destination"), processorList->data.common->Name); -- 2.17.1
This convenience macro was created for the simple cases where the length of the source string and the size of the destination buffer can be figued out with strlen() and sizeof() respectively, so we should use it wherever possible instead of open-coding parts of it. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/nwfilter_conf.c | 3 +-- src/util/virfdstream.c | 2 +- src/util/virlog.c | 5 ++--- src/util/virnetdev.c | 3 +-- src/xenconfig/xen_xl.c | 17 ++++------------- 5 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 706e803a25..36a7315880 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -966,8 +966,7 @@ ipsetValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, { const char *errmsg = NULL; - if (virStrcpy(item->u.ipset.setname, val->c, - sizeof(item->u.ipset.setname)) == NULL) { + if (virStrcpyStatic(item->u.ipset.setname, val->c) == NULL) { errmsg = _("ipset name is too long"); goto arg_err_exit; } diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c index 8189559964..f4777cfd12 100644 --- a/src/util/virfdstream.c +++ b/src/util/virfdstream.c @@ -1183,7 +1183,7 @@ int virFDStreamConnectUNIX(virStreamPtr st, goto error; sa.sun_path[0] = '\0'; } else { - if (virStrcpy(sa.sun_path, path, sizeof(sa.sun_path)) == NULL) + if (virStrcpyStatic(sa.sun_path, path) == NULL) goto error; } diff --git a/src/util/virlog.c b/src/util/virlog.c index e008dd9c54..9d569057ae 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -284,8 +284,7 @@ virLogOnceInit(void) */ r = gethostname(virLogHostname, sizeof(virLogHostname)); if (r == -1) { - ignore_value(virStrcpy(virLogHostname, - "(unknown)", sizeof(virLogHostname))); + ignore_value(virStrcpyStatic(virLogHostname, "(unknown)")); } else { NUL_TERMINATE(virLogHostname); } @@ -1027,7 +1026,7 @@ virLogOutputToJournald(virLogSourcePtr source, memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_UNIX; - if (!virStrcpy(sa.sun_path, "/run/systemd/journal/socket", sizeof(sa.sun_path))) + if (!virStrcpyStatic(sa.sun_path, "/run/systemd/journal/socket")) return; memset(&mh, 0, sizeof(mh)); diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index c20022fbc9..57ebd0ec03 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -914,8 +914,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) memset(&ifreq, 0, sizeof(ifreq)); - if (virStrncpy(ifreq.ifr_name, ifname, strlen(ifname), - sizeof(ifreq.ifr_name)) == NULL) { + if (virStrcpyStatic(ifreq.ifr_name, ifname) == NULL) { virReportSystemError(ERANGE, _("invalid interface name %s"), ifname); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index f0d9177cec..807fe621d6 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -475,15 +475,12 @@ xenParseXLVnuma(virConfPtr conf, data++; if (*data) { - size_t len; char vtoken[64]; if (STRPREFIX(str, "pnode")) { unsigned int cellid; - len = strlen(data); - if (!virStrncpy(vtoken, data, - len, sizeof(vtoken))) { + if (!virStrcpyStatic(vtoken, data)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu pnode '%s' too long for destination"), vnodeCnt, data); @@ -499,9 +496,7 @@ xenParseXLVnuma(virConfPtr conf, } pnode = cellid; } else if (STRPREFIX(str, "size")) { - len = strlen(data); - if (!virStrncpy(vtoken, data, - len, sizeof(vtoken))) { + if (!virStrcpyStatic(vtoken, data)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu size '%s' too long for destination"), vnodeCnt, data); @@ -514,9 +509,7 @@ xenParseXLVnuma(virConfPtr conf, virDomainNumaSetNodeMemorySize(numa, vnodeCnt, (kbsize * 1024)); } else if (STRPREFIX(str, "vcpus")) { - len = strlen(data); - if (!virStrncpy(vtoken, data, - len, sizeof(vtoken))) { + if (!virStrcpyStatic(vtoken, data)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu vcpus '%s' too long for destination"), vnodeCnt, data); @@ -533,9 +526,7 @@ xenParseXLVnuma(virConfPtr conf, size_t i, ndistances; unsigned int value; - len = strlen(data); - if (!virStrncpy(vtoken, data, - len, sizeof(vtoken))) { + if (!virStrcpyStatic(vtoken, data)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu vdistances '%s' too long for destination"), vnodeCnt, data); -- 2.17.1
virStrcpy() and friends are useful when the destination buffer has already been allocated, eg. as part of a struct; if we have to allocate it on the spot, VIR_STRDUP() is a better choice. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/remote/remote_daemon_dispatch.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 4a93f09a7d..e62ebfb596 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -2309,9 +2309,7 @@ remoteDispatchDomainGetSecurityLabelList(virNetServerPtr server ATTRIBUTE_UNUSED for (i = 0; i < len; i++) { size_t label_len = strlen(seclabels[i].label) + 1; remote_domain_get_security_label_ret *cur = &ret->labels.labels_val[i]; - if (VIR_ALLOC_N(cur->label.label_val, label_len) < 0) - goto cleanup; - if (virStrcpy(cur->label.label_val, seclabels[i].label, label_len) == NULL) { + if (VIR_STRDUP(cur->label.label_val, seclabels[i].label) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to copy security label")); goto cleanup; -- 2.17.1
virStrncpy() allows us to copy a substring, but if we're going to copy the entire thing it's much more convenient to use virStrcpy() instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virnetdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 57ebd0ec03..017786ce43 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2763,8 +2763,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr mcast) mcast->idx = num; break; case VIR_MCAST_TYPE_NAME_TOKEN: - if (virStrncpy(mcast->name, token, strlen(token), - VIR_MCAST_NAME_LEN) == NULL) { + if (virStrcpy(mcast->name, token, VIR_MCAST_NAME_LEN) == NULL) { virReportSystemError(EINVAL, _("Failed to parse network device name from '%s'"), buf); -- 2.17.1
The strncpy() function has this quirk where it will copy *up* to the requested number of bytes, that is, it will stop early if it encounters a NULL byte in the source string. This makes it legal to pass the size of the destination buffer (minus one byte needed for the string terminator) as the number of bytes to copy and still get something somewhat reasonable out of the operation; unfortunately, it also makes the function difficult to reason about and way too easy to misuse. We want to move away from the way strncpy() behaves and towards better defined semantics, where virStrncpy() will always copy *exactly* the number of bytes it's been asked to copy; before we can do that, though, we have to change a few of the callers. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/locking/lock_driver_sanlock.c | 3 ++- src/xenapi/xenapi_driver.c | 4 +++- src/xenconfig/xen_common.c | 14 +++++++------- src/xenconfig/xen_xl.c | 12 ++++++------ 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 345cf0a772..3f3a587541 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -1004,7 +1004,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, /* sanlock doesn't use owner_name for anything, so it's safe to take just * the first SANLK_NAME_LEN - 1 characters from vm_name */ ignore_value(virStrncpy(opt->owner_name, priv->vm_name, - SANLK_NAME_LEN - 1, SANLK_NAME_LEN)); + MIN(strlen(priv->vm_name), SANLK_NAME_LEN - 1), + SANLK_NAME_LEN)); if (state && STRNEQ(state, "")) { if ((rv = sanlock_state_to_args((char *)state, diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 42b305d316..f4375c5874 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -430,7 +430,9 @@ xenapiNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) if (xen_host_cpu_get_all(session, &host_cpu_set)) { host_cpu = host_cpu_set->contents[0]; xen_host_cpu_get_modelname(session, &modelname, host_cpu); - if (!virStrncpy(info->model, modelname, LIBVIRT_MODELNAME_LEN - 1, LIBVIRT_MODELNAME_LEN)) { + if (!virStrncpy(info->model, modelname, + MIN(strlen(modelname), LIBVIRT_MODELNAME_LEN - 1), + LIBVIRT_MODELNAME_LEN)) { virReportOOMError(); xen_host_cpu_set_free(host_cpu_set); VIR_FREE(modelname); diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 4a94127da1..815ccd030e 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -879,7 +879,7 @@ xenParseVif(char *entry, const char *vif_typename) data++; if (STRPREFIX(key, "mac=")) { - int len = nextkey ? (nextkey - data) : sizeof(mac) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("MAC address %s too big for destination"), @@ -887,7 +887,7 @@ xenParseVif(char *entry, const char *vif_typename) return NULL; } } else if (STRPREFIX(key, "bridge=")) { - int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Bridge %s too big for destination"), @@ -900,7 +900,7 @@ xenParseVif(char *entry, const char *vif_typename) if (VIR_STRNDUP(script, data, len) < 0) return NULL; } else if (STRPREFIX(key, "model=")) { - int len = nextkey ? (nextkey - data) : sizeof(model) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(model, data, len, sizeof(model)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Model %s too big for destination"), @@ -908,7 +908,7 @@ xenParseVif(char *entry, const char *vif_typename) return NULL; } } else if (STRPREFIX(key, "type=")) { - int len = nextkey ? (nextkey - data) : sizeof(type) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(type, data, len, sizeof(type)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Type %s too big for destination"), @@ -916,7 +916,7 @@ xenParseVif(char *entry, const char *vif_typename) return NULL; } } else if (STRPREFIX(key, "vifname=")) { - int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Vifname %s too big for destination"), @@ -924,14 +924,14 @@ xenParseVif(char *entry, const char *vif_typename) return NULL; } } else if (STRPREFIX(key, "ip=")) { - int len = nextkey ? (nextkey - data) : sizeof(ip) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("IP %s too big for destination"), data); return NULL; } } else if (STRPREFIX(key, "rate=")) { - int len = nextkey ? (nextkey - data) : sizeof(rate) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(rate, data, len, sizeof(rate)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("rate %s too big for destination"), data); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 807fe621d6..bc3191ad5e 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -899,7 +899,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def) data++; if (STRPREFIX(key, "type=")) { - int len = nextkey ? (nextkey - data) : sizeof(type) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(type, data, len, sizeof(type)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("type %s invalid"), @@ -907,7 +907,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def) goto skipusbctrl; } } else if (STRPREFIX(key, "version=")) { - int len = nextkey ? (nextkey - data) : sizeof(version) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(version, data, len, sizeof(version)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("version %s invalid"), @@ -917,7 +917,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def) if (virStrToLong_i(version, NULL, 16, &usbctrl_version) < 0) goto skipusbctrl; } else if (STRPREFIX(key, "ports=")) { - int len = nextkey ? (nextkey - data) : sizeof(ports) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(ports, data, len, sizeof(ports)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("version %s invalid"), @@ -1001,7 +1001,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) data++; if (STRPREFIX(key, "hostbus=")) { - int len = nextkey ? (nextkey - data) : sizeof(bus) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(bus, data, len, sizeof(bus)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("bus %s too big for destination"), @@ -1009,7 +1009,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) goto skipusb; } } else if (STRPREFIX(key, "hostaddr=")) { - int len = nextkey ? (nextkey - data) : sizeof(device) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(device, data, len, sizeof(device)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("device %s too big for destination"), @@ -1077,7 +1077,7 @@ xenParseXLChannel(virConfPtr conf, virDomainDefPtr def) data++; if (STRPREFIX(key, "connection=")) { - int len = nextkey ? (nextkey - data) : sizeof(type) - 1; + int len = nextkey ? (nextkey - data) : strlen(data); if (virStrncpy(type, data, len, sizeof(type)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("connection %s too big"), data); -- 2.17.1
Currently, the functions return a pointer to the destination buffer on success or NULL on failure. Not only does this kind of error handling look quite alien in the context of libvirt, where most functions return zero on success and a negative int on failure, but it's also somewhat pointless because unless there's been a failure the returned pointer will be the same one passed in by the user, thus offering no additional value. Change the functions so that they return an int instead. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/hacking.html.in | 2 +- src/conf/capabilities.c | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/esx/esx_driver.c | 6 +++--- src/esx/esx_vi.c | 2 +- src/esx/esx_vi_types.c | 2 +- src/hyperv/hyperv_driver.c | 2 +- src/libxl/libxl_conf.c | 2 +- src/locking/lock_driver_sanlock.c | 22 ++++++++++----------- src/lxc/lxc_driver.c | 8 ++++---- src/nwfilter/nwfilter_dhcpsnoop.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +++--- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/openvz/openvz_conf.c | 8 ++++---- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/remote/remote_driver.c | 4 ++-- src/rpc/virnetlibsshsession.c | 4 ++-- src/rpc/virnetsocket.c | 4 ++-- src/security/security_apparmor.c | 2 +- src/security/virt-aa-helper.c | 2 +- src/test/test_driver.c | 4 ++-- src/uml/uml_driver.c | 4 ++-- src/util/virfdstream.c | 4 ++-- src/util/virhostcpu.c | 4 ++-- src/util/virhostmem.c | 6 +++--- src/util/virlog.c | 2 +- src/util/virnetdev.c | 10 +++++----- src/util/virnetdevbridge.c | 6 +++--- src/util/virnetdevtap.c | 4 ++-- src/util/virnetdevvportprofile.c | 6 +++--- src/util/virstring.c | 20 +++++++++---------- src/util/virstring.h | 4 ++-- src/util/virtypedparam.c | 8 ++++---- src/xenapi/xenapi_driver.c | 6 +++--- src/xenconfig/xen_common.c | 24 +++++++++++------------ src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 20 +++++++++---------- src/xenconfig/xen_xm.c | 2 +- 41 files changed, 113 insertions(+), 115 deletions(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index fbeea3eb75..6c1a5121a4 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1134,7 +1134,7 @@ respectively. The last argument is the number of bytes available in the destination string; if a copy of the source string (including a \0) will not fit into the destination, no - bytes are copied and the routine returns NULL. Otherwise, n + bytes are copied and the routine returns <0. Otherwise, n bytes from the source are copied into the destination and a trailing \0 is appended. </p> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 7a810efa66..0f96500294 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1262,7 +1262,7 @@ virCapabilitiesGetNodeInfo(virNodeInfoPtr nodeinfo) memset(nodeinfo, 0, sizeof(*nodeinfo)); - if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL) + if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) < 0) return -1; if (virHostMemGetInfo(&memorybytes, NULL) < 0) diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index 58a50793c2..24052bf784 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -134,7 +134,7 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags) } if (virtPortProfileID && - !virStrcpyStatic(virtPort->profileID, virtPortProfileID)) { + virStrcpyStatic(virtPort->profileID, virtPortProfileID) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("profileid parameter too long")); goto error; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 36a7315880..120ca5ec14 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -966,7 +966,7 @@ ipsetValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, { const char *errmsg = NULL; - if (virStrcpyStatic(item->u.ipset.setname, val->c) == NULL) { + if (virStrcpyStatic(item->u.ipset.setname, val->c) < 0) { errmsg = _("ipset name is too long"); goto arg_err_exit; } diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index edd21b9d28..cee98ebcaf 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -914,8 +914,8 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, goto cleanup; } - if (!virStrcpyStatic(vCenterIPAddress, - potentialVCenterIPAddress)) { + if (virStrcpyStatic(vCenterIPAddress, + potentialVCenterIPAddress) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vCenter IP address %s too big for destination"), potentialVCenterIPAddress); @@ -1317,7 +1317,7 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - if (!virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string)) { + if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU Model %s too long for destination"), dynamicProperty->val->string); diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index d3c4f760ba..727d76e89d 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -185,7 +185,7 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED, curl_infotype type, if (VIR_ALLOC_N(buffer, size + 1) < 0) return 0; - if (!virStrncpy(buffer, info, size, size + 1)) { + if (virStrncpy(buffer, info, size, size + 1) < 0) { VIR_FREE(buffer); return 0; } diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 315d67d20f..7b234f0c87 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -1491,7 +1491,7 @@ esxVI_DateTime_ConvertToCalendarTime(esxVI_DateTime *dateTime, return -1; } - if (!virStrcpyStatic(value, dateTime->value)) { + if (virStrcpyStatic(value, dateTime->value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("xsd:dateTime value '%s' too long for destination"), dateTime->value); diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 6f74adf372..6bc4c099e2 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -307,7 +307,7 @@ hypervNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) } /* Fill struct */ - if (virStrcpyStatic(info->model, processorList->data.common->Name) == NULL) { + if (virStrcpyStatic(info->model, processorList->data.common->Name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU model %s too long for destination"), processorList->data.common->Name); diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index cda4eb9d31..998029dcbb 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -2365,7 +2365,7 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) goto cleanup; } - if (virStrcpyStatic(info->model, virArchToString(hostarch)) == NULL) { + if (virStrcpyStatic(info->model, virArchToString(hostarch)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("machine type %s too big for destination"), virArchToString(hostarch)); diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 3f3a587541..3e5f0e37b0 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -221,8 +221,8 @@ virLockManagerSanlockSetupLockspace(virLockManagerSanlockDriverPtr driver) VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE) < 0) goto error; - if (!virStrcpyStatic(ls.name, - VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE)) { + if (virStrcpyStatic(ls.name, + VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Lockspace path '%s' exceeded %d characters"), VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, @@ -231,7 +231,7 @@ virLockManagerSanlockSetupLockspace(virLockManagerSanlockDriverPtr driver) } ls.host_id = 0; /* Doesn't matter for initialization */ ls.flags = 0; - if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) { + if (virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Lockspace path '%s' exceeded %d characters"), path, SANLK_PATH_LEN); @@ -583,7 +583,7 @@ static int virLockManagerSanlockAddLease(virLockManagerPtr lock, res->flags = shared ? SANLK_RES_SHARED : 0; res->num_disks = 1; - if (!virStrcpy(res->name, name, SANLK_NAME_LEN)) { + if (virStrcpy(res->name, name, SANLK_NAME_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Resource name '%s' exceeds %d characters"), name, SANLK_NAME_LEN); @@ -592,7 +592,7 @@ static int virLockManagerSanlockAddLease(virLockManagerPtr lock, for (i = 0; i < nparams; i++) { if (STREQ(params[i].key, "path")) { - if (!virStrcpy(res->disks[0].path, params[i].value.str, SANLK_PATH_LEN)) { + if (virStrcpy(res->disks[0].path, params[i].value.str, SANLK_PATH_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Lease path '%s' exceeds %d characters"), params[i].value.str, SANLK_PATH_LEN); @@ -601,7 +601,7 @@ static int virLockManagerSanlockAddLease(virLockManagerPtr lock, } else if (STREQ(params[i].key, "offset")) { res->disks[0].offset = params[i].value.ul; } else if (STREQ(params[i].key, "lockspace")) { - if (!virStrcpy(res->lockspace_name, params[i].value.str, SANLK_NAME_LEN)) { + if (virStrcpy(res->lockspace_name, params[i].value.str, SANLK_NAME_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Resource lockspace '%s' exceeds %d characters"), params[i].value.str, SANLK_NAME_LEN); @@ -651,7 +651,7 @@ virLockManagerSanlockAddDisk(virLockManagerSanlockDriverPtr driver, res->num_disks = 1; if (virCryptoHashString(VIR_CRYPTO_HASH_MD5, name, &hash) < 0) goto cleanup; - if (!virStrcpy(res->name, hash, SANLK_NAME_LEN)) { + if (virStrcpy(res->name, hash, SANLK_NAME_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("MD5 hash '%s' unexpectedly larger than %d characters"), hash, (SANLK_NAME_LEN - 1)); @@ -661,16 +661,16 @@ virLockManagerSanlockAddDisk(virLockManagerSanlockDriverPtr driver, if (virAsprintf(&path, "%s/%s", driver->autoDiskLeasePath, res->name) < 0) goto cleanup; - if (!virStrcpy(res->disks[0].path, path, SANLK_PATH_LEN)) { + if (virStrcpy(res->disks[0].path, path, SANLK_PATH_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Lease path '%s' exceeds %d characters"), path, SANLK_PATH_LEN); goto cleanup; } - if (!virStrcpy(res->lockspace_name, - VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, - SANLK_NAME_LEN)) { + if (virStrcpy(res->lockspace_name, + VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, + SANLK_NAME_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Resource lockspace '%s' exceeds %d characters"), VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, SANLK_NAME_LEN); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1fc6c6a7bf..9b329269a9 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1334,8 +1334,8 @@ static int lxcNodeGetSecurityModel(virConnectPtr conn, || caps->host.secModels[0].model == NULL) goto cleanup; - if (!virStrcpy(secmodel->model, caps->host.secModels[0].model, - VIR_SECURITY_MODEL_BUFLEN)) { + if (virStrcpy(secmodel->model, caps->host.secModels[0].model, + VIR_SECURITY_MODEL_BUFLEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security model string exceeds max %d bytes"), VIR_SECURITY_MODEL_BUFLEN - 1); @@ -1343,8 +1343,8 @@ static int lxcNodeGetSecurityModel(virConnectPtr conn, goto cleanup; } - if (!virStrcpy(secmodel->doi, caps->host.secModels[0].doi, - VIR_SECURITY_DOI_BUFLEN)) { + if (virStrcpy(secmodel->doi, caps->host.secModels[0].doi, + VIR_SECURITY_DOI_BUFLEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security DOI string exceeds max %d bytes"), VIR_SECURITY_DOI_BUFLEN-1); diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 533c45f080..c7fd370598 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -595,7 +595,7 @@ virNWFilterSnoopReqNew(const char *ifkey) req->threadStatus = THREAD_STATUS_NONE; - if (virStrcpyStatic(req->ifkey, ifkey) == NULL || + if (virStrcpyStatic(req->ifkey, ifkey) < 0|| virMutexInitRecursive(&req->lock) < 0) goto err_free_req; diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index b19b07c845..c1feabe494 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -168,7 +168,7 @@ printVar(virNWFilterVarCombIterPtr vars, return -1; } - if (!virStrcpy(buf, val, bufsize)) { + if (virStrcpy(buf, val, bufsize) < 0) { const char *varName; varName = virNWFilterVarAccessGetVarName(item->varAccess); @@ -282,7 +282,7 @@ _printDataType(virNWFilterVarCombIterPtr vars, break; case DATATYPE_IPSETNAME: - if (virStrcpy(buf, item->u.ipset.setname, bufsize) == NULL) { + if (virStrcpy(buf, item->u.ipset.setname, bufsize) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer to small for ipset name")); return -1; @@ -311,7 +311,7 @@ _printDataType(virNWFilterVarCombIterPtr vars, flags = virBufferContentAndReset(&vb); - if (virStrcpy(buf, flags, bufsize) == NULL) { + if (virStrcpy(buf, flags, bufsize) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Buffer too small for IPSETFLAGS type")); VIR_FREE(flags); diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 7883afbfb2..6965af26b6 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -170,7 +170,7 @@ virNWFilterLockIface(const char *ifname) goto err_exit; } - if (virStrcpyStatic(ifaceLock->ifname, ifname) == NULL) { + if (virStrcpyStatic(ifaceLock->ifname, ifname) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("interface name %s does not fit into " "buffer "), diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 5ed2b423cb..07d0e91b24 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -274,7 +274,7 @@ openvzReadNetworkConf(virDomainDefPtr def, if (VIR_ALLOC_N(net->ifname, len+1) < 0) goto error; - if (virStrncpy(net->ifname, p, len, len+1) == NULL) { + if (virStrncpy(net->ifname, p, len, len+1) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Network ifname %s too long for destination"), p); goto error; @@ -291,7 +291,7 @@ openvzReadNetworkConf(virDomainDefPtr def, if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto error; - if (virStrncpy(net->data.bridge.brname, p, len, len+1) == NULL) { + if (virStrncpy(net->data.bridge.brname, p, len, len+1) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Bridge name %s too long for destination"), p); goto error; @@ -304,7 +304,7 @@ openvzReadNetworkConf(virDomainDefPtr def, _("Wrong length MAC address")); goto error; } - if (virStrncpy(cpy_temp, p, len, sizeof(cpy_temp)) == NULL) { + if (virStrncpy(cpy_temp, p, len, sizeof(cpy_temp)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("MAC address %s too long for destination"), p); goto error; @@ -956,7 +956,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) uuidbuf = strtok_r(NULL, "\n", &saveptr); if (iden != NULL && uuidbuf != NULL && STREQ(iden, "#UUID:")) { - if (virStrcpy(uuidstr, uuidbuf, len) == NULL) { + if (virStrcpy(uuidstr, uuidbuf, len) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid uuid %s"), uuidbuf); goto cleanup; diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 10c6ef09fa..bf08871f18 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -207,7 +207,7 @@ qemuAgentOpenUnix(const char *monitor) memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + if (virStrcpyStatic(addr.sun_path, monitor) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Agent path %s too big for destination"), monitor); goto error; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b8437463bf..58e434ba1e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4935,7 +4935,7 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev) memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - if (virStrcpyStatic(addr.sun_path, dev->data.nix.path) == NULL) { + if (virStrcpyStatic(addr.sun_path, dev->data.nix.path) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("UNIX socket path '%s' too long"), dev->data.nix.path); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 45cd51e43f..4f0bbc147d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -353,7 +353,7 @@ qemuMonitorOpenUnix(const char *monitor, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + if (virStrcpyStatic(addr.sun_path, monitor) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Monitor path %s too big for destination"), monitor); goto error; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1d94c2e42d..3b43e219e5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1608,7 +1608,7 @@ remoteNodeGetCPUStats(virConnectPtr conn, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Stats %s too big for destination"), ret.params.params_val[i].field); @@ -1672,7 +1672,7 @@ remoteNodeGetMemoryStats(virConnectPtr conn, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Stats %s too big for destination"), ret.params.params_val[i].field); diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index df819f6169..21eb9f2500 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -424,7 +424,7 @@ virNetLibsshAuthenticatePrivkeyCb(const char *prompt, virConnectCredential retr_passphrase; int cred_type; char *actual_prompt = NULL; - char *p; + int p; /* request user's key password */ if (!sess->cred || !sess->cred->cb) { @@ -459,7 +459,7 @@ virNetLibsshAuthenticatePrivkeyCb(const char *prompt, p = virStrncpy(buf, retr_passphrase.result, retr_passphrase.resultlen, len); VIR_DISPOSE_STRING(retr_passphrase.result); - if (!p) { + if (p < 0) { virReportError(VIR_ERR_LIBSSH, "%s", _("passphrase is too long for the buffer")); goto error; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 7087abec9c..fee61ace60 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -459,7 +459,7 @@ int virNetSocketNewListenUNIX(const char *path, } addr.data.un.sun_family = AF_UNIX; - if (virStrcpyStatic(addr.data.un.sun_path, path) == NULL) { + if (virStrcpyStatic(addr.data.un.sun_path, path) < 0) { virReportSystemError(ENAMETOOLONG, _("Path %s too long for unix socket"), path); goto error; @@ -690,7 +690,7 @@ int virNetSocketNewConnectUNIX(const char *path, } remoteAddr.data.un.sun_family = AF_UNIX; - if (virStrcpyStatic(remoteAddr.data.un.sun_path, path) == NULL) { + if (virStrcpyStatic(remoteAddr.data.un.sun_path, path) < 0) { virReportSystemError(ENOMEM, _("Path %s too long for unix socket"), path); goto cleanup; } diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index cb41df71a9..802ca0f14d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -530,7 +530,7 @@ AppArmorGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, } if (virStrcpy(sec->label, profile_name, - VIR_SECURITY_LABEL_BUFLEN) == NULL) { + VIR_SECURITY_LABEL_BUFLEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("error copying profile name")); goto cleanup; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 952b496f21..f104495af0 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1317,7 +1317,7 @@ vahParseArgv(vahControl * ctl, int argc, char **argv) if (strlen(optarg) > PROFILE_NAME_SIZE - 1) vah_error(ctl, 1, _("invalid UUID")); if (virStrcpy((char *)ctl->uuid, optarg, - PROFILE_NAME_SIZE) == NULL) + PROFILE_NAME_SIZE) < 0) vah_error(ctl, 1, _("error copying UUID")); break; default: diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f3ed667d68..dfca95c981 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -690,7 +690,7 @@ static char *testBuildFilename(const char *relativeTo, int totalLen = baseLen + strlen(filename) + 1; if (VIR_ALLOC_N(absFile, totalLen) < 0) return NULL; - if (virStrncpy(absFile, relativeTo, baseLen, totalLen) == NULL) { + if (virStrncpy(absFile, relativeTo, baseLen, totalLen) < 0) { VIR_FREE(absFile); return NULL; } @@ -804,7 +804,7 @@ testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) str = virXPathString("string(/node/cpu/model[1])", ctxt); if (str != NULL) { - if (virStrcpyStatic(nodeInfo->model, str) == NULL) { + if (virStrcpyStatic(nodeInfo->model, str) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Model %s too big for destination"), str); VIR_FREE(str); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index c77988f01e..425c0bac04 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -850,7 +850,7 @@ static int umlMonitorAddress(const struct uml_driver *driver, memset(addr, 0, sizeof(*addr)); addr->sun_family = AF_UNIX; - if (virStrcpyStatic(addr->sun_path, sockname) == NULL) { + if (virStrcpyStatic(addr->sun_path, sockname) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unix path %s too long for destination"), sockname); retval = -1; @@ -951,7 +951,7 @@ static int umlMonitorCommand(const struct uml_driver *driver, cmd, req.length); return -1; } - if (virStrcpyStatic(req.data, cmd) == NULL) { + if (virStrcpyStatic(req.data, cmd) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Command %s too long for destination"), cmd); return -1; diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c index f4777cfd12..cff40bad25 100644 --- a/src/util/virfdstream.c +++ b/src/util/virfdstream.c @@ -1179,11 +1179,11 @@ int virFDStreamConnectUNIX(virStreamPtr st, memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_UNIX; if (abstract) { - if (virStrcpy(sa.sun_path+1, path, sizeof(sa.sun_path)-1) == NULL) + if (virStrcpy(sa.sun_path+1, path, sizeof(sa.sun_path)-1) < 0) goto error; sa.sun_path[0] = '\0'; } else { - if (virStrcpyStatic(sa.sun_path, path) == NULL) + if (virStrcpyStatic(sa.sun_path, path) < 0) goto error; } diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 013c95bb56..060d382781 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -166,7 +166,7 @@ virHostCPUGetStatsFreeBSD(int cpuNum, for (i = 0; cpu_map[i].field != NULL; i++) { virNodeCPUStatsPtr param = ¶ms[i]; - if (virStrcpyStatic(param->field, cpu_map[i].field) == NULL) { + if (virStrcpyStatic(param->field, cpu_map[i].field) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field '%s' too long for destination"), cpu_map[i].field); @@ -933,7 +933,7 @@ virHostCPUStatsAssign(virNodeCPUStatsPtr param, const char *name, unsigned long long value) { - if (virStrcpyStatic(param->field, name) == NULL) { + if (virStrcpyStatic(param->field, name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("kernel cpu time field is too long" " for the destination")); diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index c923a1edf5..bb8b62653b 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -104,7 +104,7 @@ virHostMemGetStatsFreeBSD(virNodeMemoryStatsPtr params, } param = ¶ms[j++]; - if (virStrcpyStatic(param->field, sysctl_map[i].field) == NULL) { + if (virStrcpyStatic(param->field, sysctl_map[i].field) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field '%s' too long for destination"), sysctl_map[i].field); @@ -122,7 +122,7 @@ virHostMemGetStatsFreeBSD(virNodeMemoryStatsPtr params, "vfs.bufspace"); return -1; } - if (virStrcpyStatic(param->field, VIR_NODE_MEMORY_STATS_BUFFERS) == NULL) { + if (virStrcpyStatic(param->field, VIR_NODE_MEMORY_STATS_BUFFERS) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field '%s' too long for destination"), VIR_NODE_MEMORY_STATS_BUFFERS); @@ -224,7 +224,7 @@ virHostMemGetStatsLinux(FILE *meminfo, if (STREQ(meminfo_hdr, convp->meminfo_hdr)) { virNodeMemoryStatsPtr param = ¶ms[k++]; - if (virStrcpyStatic(param->field, convp->field) == NULL) { + if (virStrcpyStatic(param->field, convp->field) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Field kernel memory too long for destination")); goto cleanup; diff --git a/src/util/virlog.c b/src/util/virlog.c index 9d569057ae..64c0efc1bd 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1026,7 +1026,7 @@ virLogOutputToJournald(virLogSourcePtr source, memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_UNIX; - if (!virStrcpyStatic(sa.sun_path, "/run/systemd/journal/socket")) + if (virStrcpyStatic(sa.sun_path, "/run/systemd/journal/socket") < 0) return; memset(&mh, 0, sizeof(mh)); diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 017786ce43..0777ecaf3f 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -138,7 +138,7 @@ static int virNetDevSetupControlFull(const char *ifname, if (ifr && ifname) { memset(ifr, 0, sizeof(*ifr)); - if (virStrcpyStatic(ifr->ifr_name, ifname) == NULL) { + if (virStrcpyStatic(ifr->ifr_name, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), ifname); @@ -593,7 +593,7 @@ int virNetDevSetName(const char* ifname, const char *newifname) return -1; # ifdef HAVE_STRUCT_IFREQ_IFR_NEWNAME - if (virStrcpyStatic(ifr.ifr_newname, newifname) == NULL) { + if (virStrcpyStatic(ifr.ifr_newname, newifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), newifname); @@ -914,7 +914,7 @@ int virNetDevGetIndex(const char *ifname, int *ifindex) memset(&ifreq, 0, sizeof(ifreq)); - if (virStrcpyStatic(ifreq.ifr_name, ifname) == NULL) { + if (virStrcpyStatic(ifreq.ifr_name, ifname) < 0) { virReportSystemError(ERANGE, _("invalid interface name %s"), ifname); @@ -1015,7 +1015,7 @@ int virNetDevGetVLanID(const char *ifname, int *vlanid) return -1; } - if (virStrcpyStatic(vlanargs.device1, ifname) == NULL) { + if (virStrcpyStatic(vlanargs.device1, ifname) < 0) { virReportSystemError(ERANGE, _("invalid interface name %s"), ifname); @@ -2763,7 +2763,7 @@ static int virNetDevParseMcast(char *buf, virNetDevMcastEntryPtr mcast) mcast->idx = num; break; case VIR_MCAST_TYPE_NAME_TOKEN: - if (virStrcpy(mcast->name, token, VIR_MCAST_NAME_LEN) == NULL) { + if (virStrcpy(mcast->name, token, VIR_MCAST_NAME_LEN) < 0) { virReportSystemError(EINVAL, _("Failed to parse network device name from '%s'"), buf); diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index cfb7ebae99..e46ac35859 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -93,7 +93,7 @@ static int virNetDevBridgeCmd(const char *brname, return -1; } - if (virStrcpyStatic(ifd.ifd_name, brname) == NULL) { + if (virStrcpyStatic(ifd.ifd_name, brname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), brname); @@ -686,7 +686,7 @@ int virNetDevBridgeAddPort(const char *brname, struct ifbreq req; memset(&req, 0, sizeof(req)); - if (virStrcpyStatic(req.ifbr_ifsname, ifname) == NULL) { + if (virStrcpyStatic(req.ifbr_ifsname, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), ifname); @@ -756,7 +756,7 @@ int virNetDevBridgeRemovePort(const char *brname, struct ifbreq req; memset(&req, 0, sizeof(req)); - if (virStrcpyStatic(req.ifbr_ifsname, ifname) == NULL) { + if (virStrcpyStatic(req.ifbr_ifsname, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), ifname); diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 3118ca18e8..d432577c0c 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -270,7 +270,7 @@ int virNetDevTapCreate(char **ifname, ifr.ifr_flags |= IFF_VNET_HDR; # endif - if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { + if (virStrcpyStatic(ifr.ifr_name, *ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), *ifname); @@ -336,7 +336,7 @@ int virNetDevTapDelete(const char *ifname, memset(&try, 0, sizeof(struct ifreq)); try.ifr_flags = IFF_TAP|IFF_NO_PI; - if (virStrcpyStatic(try.ifr_name, ifname) == NULL) { + if (virStrcpyStatic(try.ifr_name, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%s' is too long"), ifname); diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 133d962db6..3ebf757fb3 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -389,7 +389,7 @@ virNetDevVPortProfileMerge(virNetDevVPortProfilePtr orig, orig->profileID, mods->profileID); return -1; } - if (virStrcpyStatic(orig->profileID, mods->profileID) == NULL) { + if (virStrcpyStatic(orig->profileID, mods->profileID) < 0) { /* this should never happen - it indicates mods->profileID * isn't properly null terminated. */ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -885,8 +885,8 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int break; if (tb[IFLA_IFNAME]) { - if (!virStrcpy(parent_ifname, (char*)RTA_DATA(tb[IFLA_IFNAME]), - IFNAMSIZ)) { + if (virStrcpy(parent_ifname, (char*)RTA_DATA(tb[IFLA_IFNAME]), + IFNAMSIZ) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer for root interface name is too small")); rc = -1; diff --git a/src/util/virstring.c b/src/util/virstring.c index 31e71d7535..3e2f85465f 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -774,25 +774,23 @@ virAsprintfInternal(bool report, * A safe version of strncpy. The last parameter is the number of bytes * available in the destination string, *not* the number of bytes you want * to copy. If the destination is not large enough to hold all n of the - * src string bytes plus a \0, NULL is returned and no data is copied. + * src string bytes plus a \0, <0 is returned and no data is copied. * If the destination is large enough to hold the n bytes plus \0, then the - * string is copied and a pointer to the destination string is returned. + * string is copied and 0 is returned. */ -char * +int virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) { - char *ret; - if (n > (destbytes - 1)) - return NULL; + return -1; - ret = strncpy(dest, src, n); + strncpy(dest, src, n); /* strncpy NULL terminates iff the last character is \0. Therefore * force the last byte to be \0 */ dest[n] = '\0'; - return ret; + return 0; } /** @@ -801,11 +799,11 @@ virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) * A safe version of strcpy. The last parameter is the number of bytes * available in the destination string, *not* the number of bytes you want * to copy. If the destination is not large enough to hold all n of the - * src string bytes plus a \0, NULL is returned and no data is copied. + * src string bytes plus a \0, <0 is returned and no data is copied. * If the destination is large enough to hold the source plus \0, then the - * string is copied and a pointer to the destination string is returned. + * string is copied and 0 is returned. */ -char * +int virStrcpy(char *dest, const char *src, size_t destbytes) { return virStrncpy(dest, src, strlen(src), destbytes); diff --git a/src/util/virstring.h b/src/util/virstring.h index 14948fdf1c..125fd4eede 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -127,9 +127,9 @@ void virSkipSpacesBackwards(const char *str, char **endp) bool virStringIsEmpty(const char *str); -char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) +int virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) ATTRIBUTE_RETURN_CHECK; -char *virStrcpy(char *dest, const char *src, size_t destbytes) +int virStrcpy(char *dest, const char *src, size_t destbytes) ATTRIBUTE_RETURN_CHECK; # define virStrcpyStatic(dest, src) virStrcpy((dest), (src), sizeof(dest)) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 2452628cdb..cfaa0dd955 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -86,7 +86,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) if (VIR_RESIZE_N(keys, nkeysalloc, nkeys, 1) < 0) goto cleanup; - if (virStrcpyStatic(keys[nkeys].field, name) == NULL) { + if (virStrcpyStatic(keys[nkeys].field, name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), name); goto cleanup; @@ -222,7 +222,7 @@ virTypedParameterAssign(virTypedParameterPtr param, const char *name, va_start(ap, type); - if (virStrcpyStatic(param->field, name) == NULL) { + if (virStrcpyStatic(param->field, name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), name); goto cleanup; @@ -279,7 +279,7 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, const char *name, goto cleanup; } - if (virStrcpyStatic(param->field, name) == NULL) { + if (virStrcpyStatic(param->field, name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), name); goto cleanup; @@ -1413,7 +1413,7 @@ virTypedParamsDeserialize(virTypedParameterRemotePtr remote_params, virTypedParameterRemotePtr remote_param = remote_params + i; if (virStrcpyStatic(param->field, - remote_param->field) == NULL) { + remote_param->field) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("parameter %s too big for destination"), remote_param->field); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index f4375c5874..34f9e2c717 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -430,9 +430,9 @@ xenapiNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) if (xen_host_cpu_get_all(session, &host_cpu_set)) { host_cpu = host_cpu_set->contents[0]; xen_host_cpu_get_modelname(session, &modelname, host_cpu); - if (!virStrncpy(info->model, modelname, - MIN(strlen(modelname), LIBVIRT_MODELNAME_LEN - 1), - LIBVIRT_MODELNAME_LEN)) { + if (virStrncpy(info->model, modelname, + MIN(strlen(modelname), LIBVIRT_MODELNAME_LEN - 1), + LIBVIRT_MODELNAME_LEN) < 0) { virReportOOMError(); xen_host_cpu_set_free(host_cpu_set); VIR_FREE(modelname); diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 815ccd030e..fdca9845aa 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -410,7 +410,7 @@ xenParsePCI(char *entry) return NULL; if (!(nextkey = strchr(key, ':'))) return NULL; - if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) { + if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Domain %s too big for destination"), key); return NULL; @@ -419,7 +419,7 @@ xenParsePCI(char *entry) key = nextkey + 1; if (!(nextkey = strchr(key, ':'))) return NULL; - if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) { + if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Bus %s too big for destination"), key); return NULL; @@ -428,7 +428,7 @@ xenParsePCI(char *entry) key = nextkey + 1; if (!(nextkey = strchr(key, '.'))) return NULL; - if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) { + if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Slot %s too big for destination"), key); return NULL; @@ -437,7 +437,7 @@ xenParsePCI(char *entry) key = nextkey + 1; if (strlen(key) != 1) return NULL; - if (virStrncpy(func, key, 1, sizeof(func)) == NULL) { + if (virStrncpy(func, key, 1, sizeof(func)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Function %s too big for destination"), key); return NULL; @@ -669,7 +669,7 @@ xenParseVfb(virConfPtr conf, virDomainDefPtr def) char vfb[MAX_VFB]; char *key = vfb; - if (virStrcpyStatic(vfb, list->list->str) == NULL) { + if (virStrcpyStatic(vfb, list->list->str) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("VFB %s too big for destination"), list->list->str); @@ -880,7 +880,7 @@ xenParseVif(char *entry, const char *vif_typename) if (STRPREFIX(key, "mac=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) { + if (virStrncpy(mac, data, len, sizeof(mac)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("MAC address %s too big for destination"), data); @@ -888,7 +888,7 @@ xenParseVif(char *entry, const char *vif_typename) } } else if (STRPREFIX(key, "bridge=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) { + if (virStrncpy(bridge, data, len, sizeof(bridge)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Bridge %s too big for destination"), data); @@ -901,7 +901,7 @@ xenParseVif(char *entry, const char *vif_typename) return NULL; } else if (STRPREFIX(key, "model=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(model, data, len, sizeof(model)) == NULL) { + if (virStrncpy(model, data, len, sizeof(model)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Model %s too big for destination"), data); @@ -909,7 +909,7 @@ xenParseVif(char *entry, const char *vif_typename) } } else if (STRPREFIX(key, "type=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(type, data, len, sizeof(type)) == NULL) { + if (virStrncpy(type, data, len, sizeof(type)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Type %s too big for destination"), data); @@ -917,7 +917,7 @@ xenParseVif(char *entry, const char *vif_typename) } } else if (STRPREFIX(key, "vifname=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) { + if (virStrncpy(vifname, data, len, sizeof(vifname)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Vifname %s too big for destination"), data); @@ -925,14 +925,14 @@ xenParseVif(char *entry, const char *vif_typename) } } else if (STRPREFIX(key, "ip=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) { + if (virStrncpy(ip, data, len, sizeof(ip)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("IP %s too big for destination"), data); return NULL; } } else if (STRPREFIX(key, "rate=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(rate, data, len, sizeof(rate)) == NULL) { + if (virStrncpy(rate, data, len, sizeof(rate)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("rate %s too big for destination"), data); return NULL; diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index e868c05695..af6316e7d0 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -740,7 +740,7 @@ xenParseSxprSound(virDomainDefPtr def, len = (offset2 - offset); else len = strlen(offset); - if (virStrncpy(model, offset, len, sizeof(model)) == NULL) { + if (virStrncpy(model, offset, len, sizeof(model)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Sound model %s too big for destination"), offset); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index bc3191ad5e..19b6604e05 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -480,7 +480,7 @@ xenParseXLVnuma(virConfPtr conf, if (STRPREFIX(str, "pnode")) { unsigned int cellid; - if (!virStrcpyStatic(vtoken, data)) { + if (virStrcpyStatic(vtoken, data) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu pnode '%s' too long for destination"), vnodeCnt, data); @@ -496,7 +496,7 @@ xenParseXLVnuma(virConfPtr conf, } pnode = cellid; } else if (STRPREFIX(str, "size")) { - if (!virStrcpyStatic(vtoken, data)) { + if (virStrcpyStatic(vtoken, data) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu size '%s' too long for destination"), vnodeCnt, data); @@ -509,7 +509,7 @@ xenParseXLVnuma(virConfPtr conf, virDomainNumaSetNodeMemorySize(numa, vnodeCnt, (kbsize * 1024)); } else if (STRPREFIX(str, "vcpus")) { - if (!virStrcpyStatic(vtoken, data)) { + if (virStrcpyStatic(vtoken, data) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu vcpus '%s' too long for destination"), vnodeCnt, data); @@ -526,7 +526,7 @@ xenParseXLVnuma(virConfPtr conf, size_t i, ndistances; unsigned int value; - if (!virStrcpyStatic(vtoken, data)) { + if (virStrcpyStatic(vtoken, data) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vnuma vnode %zu vdistances '%s' too long for destination"), vnodeCnt, data); @@ -900,7 +900,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def) if (STRPREFIX(key, "type=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(type, data, len, sizeof(type)) == NULL) { + if (virStrncpy(type, data, len, sizeof(type)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("type %s invalid"), data); @@ -908,7 +908,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def) } } else if (STRPREFIX(key, "version=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(version, data, len, sizeof(version)) == NULL) { + if (virStrncpy(version, data, len, sizeof(version)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("version %s invalid"), data); @@ -918,7 +918,7 @@ xenParseXLUSBController(virConfPtr conf, virDomainDefPtr def) goto skipusbctrl; } else if (STRPREFIX(key, "ports=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(ports, data, len, sizeof(ports)) == NULL) { + if (virStrncpy(ports, data, len, sizeof(ports)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("version %s invalid"), data); @@ -1002,7 +1002,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) if (STRPREFIX(key, "hostbus=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(bus, data, len, sizeof(bus)) == NULL) { + if (virStrncpy(bus, data, len, sizeof(bus)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("bus %s too big for destination"), data); @@ -1010,7 +1010,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) } } else if (STRPREFIX(key, "hostaddr=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(device, data, len, sizeof(device)) == NULL) { + if (virStrncpy(device, data, len, sizeof(device)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("device %s too big for destination"), data); @@ -1078,7 +1078,7 @@ xenParseXLChannel(virConfPtr conf, virDomainDefPtr def) if (STRPREFIX(key, "connection=")) { int len = nextkey ? (nextkey - data) : strlen(data); - if (virStrncpy(type, data, len, sizeof(type)) == NULL) { + if (virStrncpy(type, data, len, sizeof(type)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("connection %s too big"), data); goto skipchannel; diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index ef52cf4250..7b60f25ec1 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -159,7 +159,7 @@ xenParseXMDisk(char *entry, int hvm) goto error; if (virStrncpy(disk->dst, head, offset - head, - (offset - head) + 1) == NULL) { + (offset - head) + 1) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Dest file %s too big for destination"), head); goto error; -- 2.17.1
We're going to change virStrncpy() in a way that requires the source string to be NULL-terminated, so we'll no longer be able to use in this context. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/esx/esx_vi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 727d76e89d..a816c3a4f9 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -185,10 +185,8 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED, curl_infotype type, if (VIR_ALLOC_N(buffer, size + 1) < 0) return 0; - if (virStrncpy(buffer, info, size, size + 1) < 0) { - VIR_FREE(buffer); - return 0; - } + memcpy(buffer, info, size); + buffer[size] = '\0'; switch (type) { case CURLINFO_TEXT: -- 2.17.1
We finally get rid of the strncpy()-like semantics and implement our own, more sensible ones instead. As a bonus, this also fixes compilation on MinGW. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/hacking.html.in | 29 ++++++++++----------- src/util/virstring.c | 62 ++++++++++++++++++++++++++++++-------------- 2 files changed, 55 insertions(+), 36 deletions(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 6c1a5121a4..f99d143b7b 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1121,22 +1121,22 @@ <p> Do not use the strncpy function. According to the man page, it does <b>not</b> guarantee a NULL-terminated buffer, which makes - it extremely dangerous to use. Instead, use one of the - functionally equivalent functions: + it extremely dangerous to use. Instead, use one of the replacement + functions provided by libvirt: </p> <pre> virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) </pre> <p> - The first three arguments have the same meaning as for strncpy; - namely the destination, source, and number of bytes to copy, - respectively. The last argument is the number of bytes - available in the destination string; if a copy of the source - string (including a \0) will not fit into the destination, no - bytes are copied and the routine returns <0. Otherwise, n - bytes from the source are copied into the destination and a - trailing \0 is appended. + The first two arguments have the same meaning as for strncpy, + namely the destination and source of the copy operation. Unlike + strncpy, the function will always copy exactly the number of bytes + requested and make sure the destination is NULL-terminated, as the + source is required to be; sanity checks are performed to ensure the + size of the destination, as specified by the last argument, is + sufficient for the operation to succeed. On success, 0 is returned; + on failure, a value <0 is returned instead. </p> <pre> @@ -1144,10 +1144,8 @@ </pre> <p> Use this variant if you know you want to copy the entire src - string into dest. Note that this is a macro, so arguments could - be evaluated more than once. This is equivalent to - virStrncpy(dest, src, strlen(src), destbytes) - </p> + string into dest. + </p> <pre> virStrcpyStatic(char *dest, const char *src) @@ -1157,8 +1155,7 @@ string into dest <b>and</b> you know that your destination string is a static string (i.e. that sizeof(dest) returns something meaningful). Note that this is a macro, so arguments could be - evaluated more than once. This is equivalent to - virStrncpy(dest, src, strlen(src), sizeof(dest)). + evaluated more than once. </p> <pre> diff --git a/src/util/virstring.c b/src/util/virstring.c index 3e2f85465f..93fda69d7f 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -769,44 +769,66 @@ virAsprintfInternal(bool report, } /** - * virStrncpy + * virStrncpy: * - * A safe version of strncpy. The last parameter is the number of bytes - * available in the destination string, *not* the number of bytes you want - * to copy. If the destination is not large enough to hold all n of the - * src string bytes plus a \0, <0 is returned and no data is copied. - * If the destination is large enough to hold the n bytes plus \0, then the - * string is copied and 0 is returned. + * @dest: destination buffer + * @src: source buffer + * @n: number of bytes to copy + * @destbytes: number of bytes the destination can accomodate + * + * Copies the first @n bytes of @src to @dest. + * + * @src must be NULL-terminated; if successful, @dest is guaranteed to + * be NULL-terminated as well. + * + * @n must be a reasonable value, that is, it must not exceed either + * the length of @src or the size of @dest. For the latter constraint, + * the fact that @dest needs to accomodate a NULL byte in addition to + * the bytes copied from @src must be taken into account. + * + * If you want to copy *all* of @src to @dest, use virStrcpy() or + * virStrcpyStatic() instead. + * + * Returns: 0 on success, <0 on failure. */ int virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) { - if (n > (destbytes - 1)) + size_t src_len = strlen(src); + + /* As a special case, -1 means "copy the entire string". + * + * This is to avoid calling strlen() twice, once in the virStrcpy() + * wrapper and once here for bound checking purposes. */ + if (n == -1) + n = src_len; + + if (n <= 0 || n > src_len || n > (destbytes - 1)) return -1; - strncpy(dest, src, n); - /* strncpy NULL terminates iff the last character is \0. Therefore - * force the last byte to be \0 - */ + memcpy(dest, src, n); dest[n] = '\0'; return 0; } /** - * virStrcpy + * virStrcpy: + * + * @dest: destination buffer + * @src: source buffer + * @destbytes: number of bytes the destination can accomodate + * + * Copies @src to @dest. + * + * See virStrncpy() for more information. * - * A safe version of strcpy. The last parameter is the number of bytes - * available in the destination string, *not* the number of bytes you want - * to copy. If the destination is not large enough to hold all n of the - * src string bytes plus a \0, <0 is returned and no data is copied. - * If the destination is large enough to hold the source plus \0, then the - * string is copied and 0 is returned. + * Returns: 0 on success, <0 on failure. */ int virStrcpy(char *dest, const char *src, size_t destbytes) { - return virStrncpy(dest, src, strlen(src), destbytes); + return virStrncpy(dest, src, -1, destbytes); } /** -- 2.17.1
On 07/20/2018 04:44 PM, Andrea Bolognani wrote:
Changes from [v1]:
* catch a bunch more suboptimal uses of the original API;
* improve the API so that it's both more sensible and fits better with the rest of libvirt.
[v1] https://www.redhat.com/archives/libvir-list/2018-July/msg01044.html
Andrea Bolognani (8): src: Use virStrcpyStatic() to avoid truncation src: Use virStrcpyStatic() wherever possible src: Use VIR_STRDUP() wherever possible src: Use virStrcpy() wherever possible src: Don't rely on strncpy()-like behavior src: Make virStr*cpy*() functions return an int esx: Use memcpy() in esxVI_CURL_Debug() util: Improve virStrncpy() implementation
docs/hacking.html.in | 29 +++++----- src/conf/capabilities.c | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/nwfilter_conf.c | 3 +- src/esx/esx_driver.c | 8 +-- src/esx/esx_vi.c | 6 +- src/esx/esx_vi_types.c | 2 +- src/hyperv/hyperv_driver.c | 3 +- src/libxl/libxl_conf.c | 2 +- src/locking/lock_driver_sanlock.c | 25 ++++---- src/lxc/lxc_driver.c | 8 +-- src/nwfilter/nwfilter_dhcpsnoop.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/openvz/openvz_conf.c | 8 +-- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_monitor.c | 2 +- src/remote/remote_daemon_dispatch.c | 4 +- src/remote/remote_driver.c | 4 +- src/rpc/virnetlibsshsession.c | 4 +- src/rpc/virnetsocket.c | 4 +- src/security/security_apparmor.c | 2 +- src/security/virt-aa-helper.c | 2 +- src/test/test_driver.c | 4 +- src/uml/uml_driver.c | 4 +- src/util/virfdstream.c | 4 +- src/util/virhostcpu.c | 4 +- src/util/virhostmem.c | 6 +- src/util/virlog.c | 5 +- src/util/virnetdev.c | 12 ++-- src/util/virnetdevbridge.c | 6 +- src/util/virnetdevtap.c | 4 +- src/util/virnetdevvportprofile.c | 6 +- src/util/virstring.c | 70 +++++++++++++++-------- src/util/virstring.h | 4 +- src/util/virtypedparam.c | 8 +-- src/xenapi/xenapi_driver.c | 4 +- src/xenconfig/xen_common.c | 38 ++++++------ src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 41 ++++++------- src/xenconfig/xen_xm.c | 2 +- 42 files changed, 179 insertions(+), 179 deletions(-)
ACK series. Michal
participants (2)
-
Andrea Bolognani -
Michal Prívozník