On 05/03/2013 04:53 PM, Michal Privoznik wrote:
> ---
> src/util/virauth.c | 17 +--
> src/util/virauthconfig.c | 8 +-
> src/util/virbitmap.c | 9 +-
> src/util/vircgroup.c | 36 ++---
> src/util/vircommand.c | 33 ++---
> src/util/virconf.c | 34 ++---
> src/util/virdnsmasq.c | 22 +--
> src/util/virebtables.c | 34 ++---
> src/util/virebtables.h | 2 +-
> src/util/virerror.c | 19 +--
> src/util/virhash.c | 5 +-
> src/util/viridentity.c | 15 +-
> src/util/virinitctl.c | 4 +-
> src/util/viriptables.c | 4 +-
> src/util/virjson.c | 18 ++-
> src/util/virkeyfile.c | 13 +-
> src/util/virlockspace.c | 25 ++--
> src/util/virlog.c | 25 ++--
> src/util/virnetdevmacvlan.c | 16 +--
> src/util/virnetdevtap.c | 11 +-
> src/util/virnetdevvportprofile.c | 4 +-
> src/util/virobject.c | 16 ++-
> src/util/virpci.c | 14 +-
> src/util/virsexpr.c | 37 ++---
> src/util/virsocketaddr.c | 9 +-
> src/util/virstoragefile.c | 18 +--
> src/util/virstring.c | 17 +--
> src/util/virsysinfo.c | 290 ++++++++++++++++-----------------------
> src/util/virtypedparam.c | 14 +-
> src/util/viruri.c | 58 ++++----
> src/util/virutil.c | 91 +++++-------
> src/util/virxml.c | 5 +-
> 32 files changed, 356 insertions(+), 567 deletions(-)
>
> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> index c81555a..cf6a099 100644
> --- a/src/util/virbitmap.c
> +++ b/src/util/virbitmap.c
> @@ -37,6 +37,8 @@
> #include "count-one-bits.h"
> #include "virstring.h"
>
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> struct _virBitmap {
> size_t max_bit;
> size_t map_len;
> @@ -226,8 +228,11 @@ char *virBitmapFormat(virBitmapPtr bitmap)
> return NULL;
>
> cur = virBitmapNextSetBit(bitmap, -1);
> - if (cur < 0)
> - return strdup("");
> + if (cur < 0) {
> + char *ret;
> + ignore_value(VIR_STRDUP(ret, ""));
VIR_STRDUP_QUIET (and drop the #define), otherwise the function will
report an error on OOM in VIR_STRDUP and it won't report it on OOM in the
virBuffer code. (unless you plan to convert those as well)
> + return ret;
> + }
>
> start = prev = cur;
> while (prev >= 0) {
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 473d2fc..b3845ea 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -116,19 +116,14 @@ static int virCgroupCopyMounts(virCgroupPtr group,
> if (!parent->controllers[i].mountPoint)
> continue;
>
> - group->controllers[i].mountPoint =
> - strdup(parent->controllers[i].mountPoint);
> -
> - if (!group->controllers[i].mountPoint)
> + if (VIR_STRDUP(group->controllers[i].mountPoint,
> + parent->controllers[i].mountPoint) < 0)
VIR_STRDUP_QUIET
> return -ENOMEM;
>
> - if (parent->controllers[i].linkPoint) {
> - group->controllers[i].linkPoint =
> - strdup(parent->controllers[i].linkPoint);
> -
> - if (!group->controllers[i].linkPoint)
> - return -ENOMEM;
> - }
> + if (parent->controllers[i].linkPoint &&
> + VIR_STRDUP(group->controllers[i].linkPoint,
> + parent->controllers[i].linkPoint) < 0)
here too
> + return -ENOMEM;
> }
> return 0;
> }
> @@ -177,8 +172,8 @@ static int virCgroupDetectMounts(virCgroupPtr group)
> struct stat sb;
> char *tmp2;
>
> - if (!(group->controllers[i].mountPoint = strdup(entry.mnt_dir)))
> - goto no_memory;
> + if (VIR_STRDUP(group->controllers[i].mountPoint, entry.mnt_dir) < 0)
> + goto error;
VIR_STRDUP_QUIET and goto no_memory;
no_memory doesn't report an error here, it sets errno.
>
> tmp2 = strrchr(entry.mnt_dir, '/');
> if (!tmp2) {
> @@ -239,7 +234,7 @@ static int virCgroupCopyPlacement(virCgroupPtr group,
> continue;
>
> if (path[0] == '/') {
> - if (!(group->controllers[i].placement = strdup(path)))
> + if (VIR_STRDUP(group->controllers[i].placement, path) < 0)
> return -ENOMEM;
> } else {
> /*
> @@ -821,7 +816,7 @@ static int virCgroupNew(const char *path,
> }
>
> if (path[0] == '/' || !parent) {
> - if (!((*group)->path = strdup(path))) {
> + if (VIR_STRDUP((*group)->path, path) < 0) {
> rc = -ENOMEM;
> goto err;
> }
VIR_STRDUP_QUIET in both hunks above.
> @@ -1027,7 +1022,7 @@ static int virCgroupAddTaskStrController(virCgroupPtr group,
> int rc = 0;
> char *endp;
>
> - if (!(str = strdup(pidstr)))
> + if (VIR_STRDUP(str, pidstr) < 0)
> return -1;
VIR_STRDUP_QUIET
return -ENOMEM;
>
> cur = str;
> @@ -1253,7 +1248,7 @@ int virCgroupNewPartition(const char *path,
>
> if (STRNEQ(newpath, "/")) {
> char *tmp;
> - if (!(parentPath = strdup(newpath))) {
> + if (VIR_STRDUP(parentPath, newpath) < 0) {
VIR_STRDUP_QUIET
> rc = -ENOMEM;
> goto cleanup;
> }
> @@ -2543,12 +2538,11 @@ static char *virCgroupIdentifyRoot(virCgroupPtr group)
> }
>
All this:
> tmp[0] = '\0';
> - ret = strdup(group->controllers[i].mountPoint);
> - tmp[0] = '/';
> - if (!ret) {
> - virReportOOMError();
> + if (VIR_STRDUP(ret, group->controllers[i].mountPoint) < 0) {
> + tmp[0] = '/';
> return NULL;
> }
> + tmp[0] = '/';
can be replaced by:
ignore_value(VIR_STRNDUP(ret, group->controllers[i].mountPoint,
tmp - group->controllers[i].mountPoint));
althought that would probably work better as a separate patch.
> return ret;
> }
>
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index f6f27d9..eecda58 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -946,9 +946,8 @@ virCommandSetPidFile(virCommandPtr cmd, const char *pidfile)
> return;
>
> VIR_FREE(cmd->pidfile);
> - if (!(cmd->pidfile = strdup(pidfile))) {
> + if (VIR_STRDUP(cmd->pidfile, pidfile) < 0)
> cmd->has_error = ENOMEM;
> - }
> }
>
>
> @@ -1049,7 +1048,7 @@ virCommandSetSELinuxLabel(virCommandPtr cmd,
>
> #if defined(WITH_SECDRIVER_SELINUX)
> VIR_FREE(cmd->seLinuxLabel);
> - if (label && !(cmd->seLinuxLabel = strdup(label)))
> + if (label && VIR_STRDUP(cmd->seLinuxLabel, label) < 0)
> cmd->has_error = ENOMEM;
> #endif
> return;
> @@ -1074,7 +1073,7 @@ virCommandSetAppArmorProfile(virCommandPtr cmd,
>
> #if defined(WITH_SECDRIVER_APPARMOR)
> VIR_FREE(cmd->appArmorProfile);
> - if (profile && !(cmd->appArmorProfile = strdup(profile)))
> + if (profile && VIR_STRDUP(cmd->appArmorProfile, profile) < 0)
> cmd->has_error = ENOMEM;
> #endif
> return;
> @@ -1205,7 +1204,7 @@ virCommandAddEnvString(virCommandPtr cmd, const char *str)
> if (!cmd || cmd->has_error)
> return;
>
> - if (!(env = strdup(str))) {
> + if (VIR_STRDUP(env, str) < 0) {
> cmd->has_error = ENOMEM;
> return;
> }
> @@ -1309,7 +1308,7 @@ virCommandAddArg(virCommandPtr cmd, const char *val)
> if (!cmd || cmd->has_error)
> return;
>
> - if (!(arg = strdup(val))) {
> + if (VIR_STRDUP(arg, val) < 0) {
> cmd->has_error = ENOMEM;
> return;
> }
> @@ -1350,11 +1349,11 @@ virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf)
> }
>
> cmd->args[cmd->nargs] = virBufferContentAndReset(buf);
> - if (!cmd->args[cmd->nargs])
> - cmd->args[cmd->nargs] = strdup("");
> if (!cmd->args[cmd->nargs]) {
> - cmd->has_error = ENOMEM;
> - return;
> + if (VIR_STRDUP(cmd->args[cmd->nargs], "") < 0) {
> + cmd->has_error = ENOMEM;
> + return;
> + }
> }
> cmd->nargs++;
> }
> @@ -1440,8 +1439,9 @@ virCommandAddArgSet(virCommandPtr cmd, const char *const*vals)
>
> narg = 0;
> while (vals[narg] != NULL) {
> - char *arg = strdup(vals[narg++]);
> - if (!arg) {
> + char *arg;
> +
> + if (VIR_STRDUP(arg, vals[narg++]) < 0) {
> cmd->has_error = ENOMEM;
> return;
> }
> @@ -1481,8 +1481,7 @@ virCommandAddArgList(virCommandPtr cmd, ...)
> char *arg = va_arg(list, char *);
> if (!arg)
> break;
> - arg = strdup(arg);
> - if (!arg) {
> + if (VIR_STRDUP(arg, arg) < 0) {
> cmd->has_error = ENOMEM;
> va_end(list);
> return;
> @@ -1511,8 +1510,7 @@ virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd)
> cmd->has_error = -1;
> VIR_DEBUG("cannot set directory twice");
> } else {
> - cmd->pwd = strdup(pwd);
> - if (!cmd->pwd)
> + if (VIR_STRDUP(cmd->pwd, pwd) < 0)
> cmd->has_error = ENOMEM;
> }
> }
> @@ -1539,8 +1537,7 @@ virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf)
> return;
> }
>
> - cmd->inbuf = strdup(inbuf);
> - if (!cmd->inbuf)
> + if (VIR_STRDUP(cmd->inbuf, inbuf) < 0)
> cmd->has_error = ENOMEM;
> }
>
VIR_STRDUP_QUIET for all of the above.
OOM Error for these is reported in virCommandRun.
> diff --git a/src/util/virhash.c b/src/util/virhash.c
> index 2fe8751..bb708fc 100644
> --- a/src/util/virhash.c
> +++ b/src/util/virhash.c
> @@ -85,7 +86,9 @@ static bool virHashStrEqual(const void *namea, const void *nameb)
>
> static void *virHashStrCopy(const void *name)
> {
> - return strdup(name);
> + char *ret;
> + ignore_value(VIR_STRDUP(ret, name));
> + return ret;
> }
>
VIR_STRDUP_QUIET, as the error gets reported in virHashAddOrUpdateEntry.
> static void virHashStrFree(void *name)
> diff --git a/src/util/viriptables.c b/src/util/viriptables.c
> index 06a1356..88c3bcd 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -118,10 +118,10 @@ iptRulesNew(const char *table,
> if (VIR_ALLOC(rules) < 0)
> return NULL;
>
> - if (!(rules->table = strdup(table)))
> + if (VIR_STRDUP(rules->table, table) < 0)
> goto error;
>
> - if (!(rules->chain = strdup(chain)))
> + if (VIR_STRDUP(rules->chain, chain) < 0)
> goto error;
>
> return rules;
VIR_STRDUP_QUIET, unless you plan to adjust the comment for iptablesContextNew
and remove the extra OOMError from bridge_driver.c
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 92138d3..66376c1 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -107,7 +107,7 @@ virJSONValuePtr virJSONValueNewString(const char *data)
> return NULL;
>
> val->type = VIR_JSON_TYPE_STRING;
> - if (!(val->data.string = strdup(data))) {
> + if (VIR_STRDUP(val->data.string, data) < 0) {
> VIR_FREE(val);
> return NULL;
> }
> @@ -126,7 +126,7 @@ virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length)
> return NULL;
>
> val->type = VIR_JSON_TYPE_STRING;
> - if (!(val->data.string = strndup(data, length))) {
> + if (VIR_STRNDUP(val->data.string, data, length) < 0) {
> VIR_FREE(val);
> return NULL;
> }
> @@ -142,7 +142,7 @@ static virJSONValuePtr virJSONValueNewNumber(const char *data)
> return NULL;
>
> val->type = VIR_JSON_TYPE_NUMBER;
> - if (!(val->data.number = strdup(data))) {
> + if (VIR_STRDUP(val->data.number, data) < 0) {
> VIR_FREE(val);
> return NULL;
> }
> @@ -269,7 +269,7 @@ int virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONVal
> if (virJSONValueObjectHasKey(object, key))
> return -1;
>
> - if (!(newkey = strdup(key)))
> + if (VIR_STRDUP(newkey, key) < 0)
> return -1;
>
> if (VIR_REALLOC_N(object->data.object.pairs,
> @@ -751,10 +751,10 @@ static int virJSONParserHandleNumber(void *ctx,
> yajl_size_t l)
> {
> virJSONParserPtr parser = ctx;
> - char *str = strndup(s, l);
> + char *str;
> virJSONValuePtr value;
>
> - if (!str)
> + if (VIR_STRNDUP(str, s, l) < 0)
> return -1;
> value = virJSONValueNewNumber(str);
> VIR_FREE(str);
> @@ -808,8 +808,7 @@ static int virJSONParserHandleMapKey(void *ctx,
> state = &parser->state[parser->nstate-1];
> if (state->key)
> return 0;
> - state->key = strndup((const char *)stringVal, stringLen);
> - if (!state->key)
> + if (VIR_STRNDUP(state->key, (const char *)stringVal, stringLen) < 0)
> return 0;
> return 1;
> }
All these are expected by the callers to be quiet.
> @@ -1094,8 +1093,7 @@ char *virJSONValueToString(virJSONValuePtr object,
> goto cleanup;
> }
>
> - if (!(ret = strdup((const char *)str)))
> - virReportOOMError();
> + ignore_value(VIR_STRDUP(ret, (const char *)str));
>
> cleanup:
> yajl_gen_free(g);
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index eee9ddc..2ed0b10 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -557,8 +557,7 @@ virLogDefineFilter(const char *match,
> }
> }
>
> - mdup = strdup(match);
> - if (mdup == NULL) {
> + if (VIR_STRDUP(mdup, match) < 0) {
> i = -1;
> goto cleanup;
> }
> @@ -664,13 +663,9 @@ virLogDefineOutput(virLogOutputFunc f,
> if (f == NULL)
> return -1;
>
> - if (dest == VIR_LOG_TO_SYSLOG || dest == VIR_LOG_TO_FILE) {
> - if (name == NULL)
> - return -1;
> - ndup = strdup(name);
> - if (ndup == NULL)
> - return -1;
> - }
> + if ((dest == VIR_LOG_TO_SYSLOG || dest == VIR_LOG_TO_FILE) &&
> + (!name || VIR_STRDUP(ndup, name) < 0))
> + return -1;
>
> virLogLock();
> if (VIR_REALLOC_N(virLogOutputs, virLogNbOutputs + 1)) {
> @@ -1047,8 +1042,7 @@ virLogAddOutputToSyslog(virLogPriority priority,
> * ident needs to be kept around on Solaris
> */
> VIR_FREE(current_ident);
> - current_ident = strdup(ident);
> - if (current_ident == NULL)
> + if (VIR_STRDUP(current_ident, ident) < 0)
> return -1;
>
> openlog(current_ident, 0, 0);
> @@ -1329,8 +1323,7 @@ virLogParseOutputs(const char *outputs)
> if (str == cur)
> goto cleanup;
> #if HAVE_SYSLOG_H
> - name = strndup(str, cur - str);
> - if (name == NULL)
> + if (VIR_STRNDUP(name, str, cur - str) < 0)
> goto cleanup;
> if (virLogAddOutputToSyslog(prio, name) == 0)
> count++;
> @@ -1346,8 +1339,7 @@ virLogParseOutputs(const char *outputs)
> cur++;
> if (str == cur)
> goto cleanup;
> - name = strndup(str, cur - str);
> - if (name == NULL)
> + if (VIR_STRNDUP(name, str, cur - str) < 0)
> goto cleanup;
> if (virFileAbsPath(name, &abspath) < 0) {
> VIR_FREE(name);
> @@ -1424,8 +1416,7 @@ virLogParseFilters(const char *filters)
> cur++;
> if (str == cur)
> goto cleanup;
> - name = strndup(str, cur - str);
> - if (name == NULL)
> + if (VIR_STRNDUP(name, str, cur - str) < 0)
> goto cleanup;
> if (virLogDefineFilter(name, prio, flags) >= 0)
> count++;
All the calls in this file should be quiet too.
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index 0c4fcbd..93bf798 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -764,14 +765,14 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
> if (virtPortProfile && virNetlinkEventServiceIsRunning(NETLINK_ROUTE)) {
> if (VIR_ALLOC(calld) < 0)
> goto memory_error;
> - if ((calld->cr_ifname = strdup(ifname)) == NULL)
> - goto memory_error;
> + if (VIR_STRDUP(calld->cr_ifname, ifname) < 0)
> + goto error;
> if (VIR_ALLOC(calld->virtPortProfile) < 0)
> goto memory_error;
> memcpy(calld->virtPortProfile, virtPortProfile, sizeof(*virtPortProfile));
> virMacAddrSet(&calld->macaddress, macaddress);
> - if ((calld->linkdev = strdup(linkdev)) == NULL)
> - goto memory_error;
> + if (VIR_STRDUP(calld->linkdev, linkdev) < 0)
> + goto error;
double space ^^
> memcpy(calld->vmuuid, vmuuid, sizeof(calld->vmuuid));
>
> calld->vmOp = vmOp;
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index 93e37e4..72a5248 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -28,6 +28,7 @@
> #include "viratomic.h"
> #include "virerror.h"
> #include "virlog.h"
> +#include "virstring.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -37,7 +38,7 @@ struct _virClass {
> virClassPtr parent;
>
> unsigned int magic;
> - const char *name;
> + char *name;
> size_t objectSize;
>
> virObjectDisposeCallback dispose;
> @@ -131,21 +132,22 @@ virClassPtr virClassNew(virClassPtr parent,
> return NULL;
> }
>
> - if (VIR_ALLOC(klass) < 0)
> - goto no_memory;
> + if (VIR_ALLOC(klass) < 0) {
> + virReportOOMError();
> + goto error;
> + }
>
> klass->parent = parent;
> - if (!(klass->name = strdup(name)))
> - goto no_memory;
> + if (VIR_STRDUP(klass->name, name) < 0)
> + goto error;
Wouldn't it be nicer to do it with a temporary variable, so we can keep the const?
> klass->magic = virAtomicIntInc(&magicCounter);
> klass->objectSize = objectSize;
> klass->dispose = dispose;
>
> return klass;
>
> -no_memory:
> +error:
> VIR_FREE(klass);
> - virReportOOMError();
> return NULL;
> }
>
> diff --git a/src/util/virsexpr.c b/src/util/virsexpr.c
> index 23b6781..b17c0d4 100644
> --- a/src/util/virsexpr.c
> +++ b/src/util/virsexpr.c
> @@ -119,16 +119,10 @@ sexpr_string(const char *str, ssize_t len)
> if (ret == NULL)
> return ret;
> ret->kind = SEXPR_VALUE;
> - if (len > 0) {
> - ret->u.value = strndup(str, len);
> - } else {
> - ret->u.value = strdup(str);
> - }
>
> - if (ret->u.value == NULL) {
> + if ((len > 0 && VIR_STRNDUP(ret->u.value, str, len) < 0) ||
> + (len <= 0 && VIR_STRDUP(ret->u.value, str) < 0))
> VIR_FREE(ret);
> - return NULL;
> - }
>
> return ret;
> }
This function was quiet before, but it doesn't seem to be used anywhere.
> @@ -527,13 +511,10 @@ int sexpr_node_copy(const struct sexpr *sexpr, const char *node, char **dst)
> {
> const char *val = sexpr_node(sexpr, node);
>
> - if (val && *val) {
> - *dst = strdup(val);
> - if (!(*dst))
> - return -1;
> - } else {
> - *dst = NULL;
> - }
> + if (val && *val)
> + return VIR_STRDUP(*dst, val);
> +
> + *dst = NULL;
> return 0;
> }
>
Callers report the OOM error for this function, but it seems it would be
better to leave it here and clean them up.
> diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
> index 2efe634..7fd2537 100644
> --- a/src/util/virsysinfo.c
> +++ b/src/util/virsysinfo.c
> @@ -144,32 +144,26 @@ virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret)
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol &&
> - ((ret->system_family = strndup(cur, eol - cur)) == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(ret->system_family, cur, eol - cur) < 0)
> + return -1;
>
> if ((cur = strstr(base, "model")) != NULL) {
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol && ((ret->system_serial = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(ret->system_serial, cur, eol - cur) < 0)
> + return -1;
> }
>
> if ((cur = strstr(base, "machine")) != NULL) {
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol && ((ret->system_version = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(ret->system_version, cur, eol - cur) < 0)
> + return -1;
> }
>
> return 0;
> -
> -no_memory:
> - return -1;
> }
>
> static int
> @@ -185,34 +179,31 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret)
> cur = strchr(base, ':') + 1;
>
> if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
> - goto no_memory;
> + return -1;
> }
> processor = &ret->processor[ret->nprocessor - 1];
>
> virSkipSpaces(&cur);
> - if (eol &&
> - ((processor->processor_socket_destination = strndup
> - (cur, eol - cur)) == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(processor->processor_socket_destination,
> + cur, eol - cur) < 0)
> + return -1;
>
> if ((cur = strstr(base, "cpu")) != NULL) {
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol &&
> - ((processor->processor_type = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(processor->processor_type,
> + cur, eol - cur) < 0)
> + return -1;
> }
>
> if ((cur = strstr(base, "revision")) != NULL) {
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol &&
> - ((processor->processor_version = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNUDP(processor->processor_version,
typo ^^
> + cur, eol - cur) < 0)
> + return -1;
> }
>
> base = cur;
You have replaced all occurences of goto no_memory with return -1, but you
left the no_memory label here.
> @@ -270,32 +261,26 @@ virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret)
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol &&
> - ((ret->system_family = strndup(cur, eol - cur)) == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(ret->system_family, cur, eol - cur) < 0)
> + return -1;
>
> if ((cur = strstr(base, "model")) != NULL) {
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol && ((ret->system_serial = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(ret->system_serial, cur, eol - cur) < 0)
> + return -1;
> }
>
> if ((cur = strstr(base, "machine")) != NULL) {
> cur = strchr(cur, ':') + 1;
> eol = strchr(cur, '\n');
> virSkipSpaces(&cur);
> - if (eol && ((ret->system_version = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(ret->system_version, cur, eol - cur) < 0)
> + return -1;
> }
>
> return 0;
> -
> -no_memory:
> - return -1;
> }
>
> static int
> @@ -313,10 +298,8 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret)
> eol = strchr(base, '\n');
> cur = strchr(base, ':') + 1;
> virSkipSpaces(&cur);
> - if (eol &&
> - ((processor_type = strndup(cur, eol - cur))
> - == NULL))
> - goto no_memory;
> + if (eol && VIR_STRNDUP(processor_type, cur, eol - cur) < 0)
> + goto error;
> base = cur;
>
> while ((tmp_base = strstr(base, "processor")) != NULL) {
> @@ -325,19 +308,20 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret)
> cur = strchr(base, ':') + 1;
>
> if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
> - goto no_memory;
> + virReportOOMError();
Unrelated, but missing.
> + goto error;
> }
> processor = &ret->processor[ret->nprocessor - 1];
>
> virSkipSpaces(&cur);
> if (eol &&
> - ((processor->processor_socket_destination = strndup
> - (cur, eol - cur)) == NULL))
> - goto no_memory;
> + VIR_STRNDUP(processor->processor_socket_destination,
> + cur, eol - cur) < 0)
> + goto error;
>
> if (processor_type &&
> - !(processor->processor_type = strdup(processor_type)))
> - goto no_memory;
> + VIR_STRDUP(processor->processor_type, processor_type) < 0)
> + goto error;
>
> base = cur;
> }
>
virSysinfoRead for non-windows x86 seems to be the only one reporting OOM
errors, which should be deleted after you switch VIR_ALLOC to report errors
too and the other architectures might be missing some OOM errors, like arm in
the hunk above.
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 982d4a3..d3d77b2 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -547,11 +547,8 @@ virFileResolveLinkHelper(const char *linkpath,
> if (lstat(linkpath, &st) < 0)
> return -1;
>
> - if (!S_ISLNK(st.st_mode)) {
> - if (!(*resultpath = strdup(linkpath)))
> - return -1;
> - return 0;
> - }
> + if (!S_ISLNK(st.st_mode))
> + return VIR_STRDUP(*resultpath, linkpath);
> }
>
> *resultpath = canonicalize_file_name(linkpath);
This function should return -1 and set errno on error, not report an error.
> @@ -623,9 +620,11 @@ char *virFindFileInPath(const char *file)
> * copy of that path, after validating that it is executable
> */
> if (IS_ABSOLUTE_FILE_NAME(file)) {
> - if (virFileIsExecutable(file))
> - return strdup(file);
> - else
> + if (virFileIsExecutable(file)) {
> + char *ret;
> + ignore_value(VIR_STRDUP(ret, file));
VIR_STRDUP_QUIET
> + return ret;
> + } else
> return NULL;
> }
>
> @@ -641,7 +640,7 @@ char *virFindFileInPath(const char *file)
> /* copy PATH env so we can tweak it */
> path = getenv("PATH");
>
> - if (path == NULL || (path = strdup(path)) == NULL)
> + if (!path|| VIR_STRDUP(path, path) < 0)
missing space; VIR_STRDUP_QUIET
> return NULL;
>
> /* for each path segment, append the file to search for and test for
> @@ -1296,7 +1295,7 @@ virFileMakePathWithMode(const char *path,
> int ret = -1;
> char *tmp;
>
> - if ((tmp = strdup(path)) == NULL)
> + if (VIR_STRDUP(tmp, path) < 0)
> goto cleanup;
This one should be quiet and set errno.
>
> ret = virFileMakePathHelper(tmp, mode);
> @@ -1503,8 +1502,7 @@ int virFileAbsPath(const char *path, char **abspath)
> char *buf;
>
> if (path[0] == '/') {
> - if (!(*abspath = strdup(path)))
> - return -1;
> + return VIR_STRDUP(*abspath, path);
VIR_STRDUP_QUIET
> } else {
> buf = getcwd(NULL, 0);
> if (buf == NULL)
> @@ -2810,7 +2782,8 @@ char *virFileFindMountPoint(const char *type)
>
> while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) {
> if (STREQ(mb.mnt_type, type)) {
> - ret = strdup(mb.mnt_dir);
> + if (VIR_STRDUP(ret, mb.mnt_dir) < 0)
VIR_STRDUP_QUIET
> + errno = ENOMEM;
> goto cleanup;
> }
> }
ACK (but I only compiled it on amd64 linux)
Jan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list