Re: [libvirt] [PATCH v3 25/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/util/*

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