6:44 a.m.
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) < 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC? It would have three nice features:
1) sizeof(code base) gets lower. A lot lower.
2) even for callers which don't follow the schema described
above, there is no harm reporting so serious error in the
logs. No matter that the callee may fall back and return
success.
3) Removing virReportOOMError() from the schema does not need to
be done at once, but can be split into several patches. In the
worst case scenario - the error gets reported twice.
But before I start working on other areas of code, I want to make
sure there's an agreement if this is even desired. As an example,
how much the code base will lose on weight, I've done the
conversion under src/util/:
30 files changed, 81 insertions(+), 221 deletions(-)
Michal Privoznik (2):
viralloc: Report OOM error on failure
util: Don't report OOM twice
src/util/iohelper.c | 4 +---
src/util/viralloc.c | 23 ++++++++++++++++++-----
src/util/viralloc.h | 13 ++++++++-----
src/util/virauthconfig.c | 8 ++------
src/util/vircommand.c | 13 +++----------
src/util/virconf.c | 10 ++--------
src/util/virdnsmasq.c | 19 +++++++------------
src/util/virerror.c | 2 +-
src/util/vireventpoll.c | 4 +---
src/util/virfile.c | 4 +---
src/util/virhash.c | 10 ++--------
src/util/virkeyfile.c | 4 +---
src/util/virlockspace.c | 11 +++--------
src/util/virnetdev.c | 4 +---
src/util/virnetdevbandwidth.c | 12 +++---------
src/util/virnetdevmacvlan.c | 4 ++--
src/util/virnetdevvlan.c | 4 +---
src/util/virnetdevvportprofile.c | 4 +---
src/util/virnetlink.c | 8 ++------
src/util/virobject.c | 9 ++++-----
src/util/virpci.c | 13 +++----------
src/util/virprocess.c | 4 +---
src/util/virsexpr.c | 4 +---
src/util/virstoragefile.c | 24 ++++++------------------
src/util/virstring.c | 5 +++--
src/util/virsysinfo.c | 8 +++-----
src/util/virthreadpool.c | 21 +++++----------------
src/util/virtime.c | 8 ++------
src/util/virtypedparam.c | 36 +++++++++---------------------------
src/util/viruri.c | 2 +-
src/util/virusb.c | 8 ++------
src/util/virutil.c | 36 ++++++++----------------------------
src/util/virxml.c | 1 -
33 files changed, 108 insertions(+), 232 deletions(-)
--
1.8.1.5
6:44 a.m.
New subject: [libvirt] [PATCH RFC 1/2] viralloc: Report OOM error on failure
In nearly all cases of calling VIR_ALLOC*, VIR_REALLOC_N,
VIR_EXPAND_N, VIR_RESIZE_N, VIR_*_ELEMENT etc. we want to
report OOM error, so our source code base is full of:
if (VIR_ALLOC(somePtr) < 0) {
virReportOOMError();
goto cleanup;
}
or similar. Moreover, for those few cases where we don't want to
report OOM error (e.g. virReportOOMError() itself) a new
VIR_ALLOC_NOOOM macro is being introduced.
---
src/util/viralloc.c | 23 ++++++++++++++++++-----
src/util/viralloc.h | 13 ++++++++-----
src/util/virerror.c | 2 +-
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index 342b0eb..60c33d2 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -24,8 +24,11 @@
#include <stdlib.h>
#include "viralloc.h"
+#include "virerror.h"
#include "virlog.h"
+#define VIR_FROM_THIS VIR_FROM_NONE
+
#if TEST_OOM
static int testMallocNext = 0;
static int testMallocFailFirst = 0;
@@ -105,6 +108,7 @@ void virAllocTestHook(void (*func)(int, void*) ATTRIBUTE_UNUSED,
* virAlloc:
* @ptrptr: pointer to pointer for address of allocated memory
* @size: number of bytes to allocate
+ * @report: report OOM error
*
* Allocate 'size' bytes of memory. Return the address of the
* allocated memory in 'ptrptr'. The newly allocated memory is
@@ -112,7 +116,7 @@ void virAllocTestHook(void (*func)(int, void*) ATTRIBUTE_UNUSED,
*
* Returns -1 on failure to allocate, zero on success
*/
-int virAlloc(void *ptrptr, size_t size)
+int virAlloc(void *ptrptr, size_t size, bool report)
{
#if TEST_OOM
if (virAllocTestFail()) {
@@ -122,8 +126,11 @@ int virAlloc(void *ptrptr, size_t size)
#endif
*(void **)ptrptr = calloc(1, size);
- if (*(void **)ptrptr == NULL)
+ if (*(void **)ptrptr == NULL) {
+ if (report)
+ virReportOOMError();
return -1;
+ }
return 0;
}
@@ -150,8 +157,10 @@ int virAllocN(void *ptrptr, size_t size, size_t count)
#endif
*(void**)ptrptr = calloc(count, size);
- if (*(void**)ptrptr == NULL)
+ if (*(void**)ptrptr == NULL) {
+ virReportOOMError();
return -1;
+ }
return 0;
}
@@ -182,8 +191,10 @@ int virReallocN(void *ptrptr, size_t size, size_t count)
return -1;
}
tmp = realloc(*(void**)ptrptr, size * count);
- if (!tmp && (size * count))
+ if (!tmp && (size * count)) {
+ virReportOOMError();
return -1;
+ }
*(void**)ptrptr = tmp;
return 0;
}
@@ -422,8 +433,10 @@ int virAllocVar(void *ptrptr, size_t struct_size, size_t
element_size, size_t co
alloc_size = struct_size + (element_size * count);
*(void **)ptrptr = calloc(1, alloc_size);
- if (*(void **)ptrptr == NULL)
+ if (*(void **)ptrptr == NULL) {
+ virReportOOMError();
return -1;
+ }
return 0;
}
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7be7f82..30ffe15 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -46,7 +46,7 @@
/* Don't call these directly - use the macros below */
-int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK
+int virAlloc(void *ptrptr, size_t size, bool report) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
@@ -76,13 +76,16 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* VIR_ALLOC:
* @ptr: pointer to hold address of allocated memory
*
- * Allocate sizeof(*ptr) bytes of memory and store
- * the address of allocated memory in 'ptr'. Fill the
- * newly allocated memory with zeros.
+ * Allocate sizeof(*ptr) bytes of memory and store the
+ * address of allocated memory in 'ptr'. Fill the newly
+ * allocated memory with zeros. If there's a failure,
+ * OOM error is reported. The VIR_ALLOC_NOOOM macro
+ * behaves the same except the OOM error reporting.
*
* Returns -1 on failure, 0 on success
*/
-# define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)))
+# define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)), true)
+# define VIR_ALLOC_NOOOM(ptr) virAlloc(&(ptr), sizeof(*(ptr)), true)
/**
* VIR_ALLOC_N:
diff --git a/src/util/virerror.c b/src/util/virerror.c
index c30642a..c033129 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -204,7 +204,7 @@ virLastErrorObject(void)
virErrorPtr err;
err = virThreadLocalGet(&virLastErr);
if (!err) {
- if (VIR_ALLOC(err) < 0)
+ if (VIR_ALLOC_NOOOM(err) < 0)
return NULL;
if (virThreadLocalSet(&virLastErr, err) < 0)
VIR_FREE(err);
--
1.8.1.5
12:27 p.m.
New subject: [libvirt] [PATCH RFC 1/2] viralloc: Report OOM error on failure
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
In nearly all cases of calling VIR_ALLOC*, VIR_REALLOC_N,
VIR_EXPAND_N, VIR_RESIZE_N, VIR_*_ELEMENT etc. we want to
report OOM error, so our source code base is full of:
if (VIR_ALLOC(somePtr) < 0) {
virReportOOMError();
goto cleanup;
}
or similar. Moreover, for those few cases where we don't want to
report OOM error (e.g. virReportOOMError() itself) a new
VIR_ALLOC_NOOOM macro is being introduced.
---
[...]
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7be7f82..30ffe15 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -46,7 +46,7 @@
/* Don't call these directly - use the macros below */
-int virAlloc(void *ptrptr, size_t size) ATTRIBUTE_RETURN_CHECK
+int virAlloc(void *ptrptr, size_t size, bool report) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
int virAllocN(void *ptrptr, size_t size, size_t count) ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1);
@@ -76,13 +76,16 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* VIR_ALLOC:
* @ptr: pointer to hold address of allocated memory
*
- * Allocate sizeof(*ptr) bytes of memory and store
- * the address of allocated memory in 'ptr'. Fill the
- * newly allocated memory with zeros.
+ * Allocate sizeof(*ptr) bytes of memory and store the
+ * address of allocated memory in 'ptr'. Fill the newly
+ * allocated memory with zeros. If there's a failure,
+ * OOM error is reported. The VIR_ALLOC_NOOOM macro
+ * behaves the same except the OOM error reporting.
*
* Returns -1 on failure, 0 on success
*/
-# define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)))
+# define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr)), true)
+# define VIR_ALLOC_NOOOM(ptr) virAlloc(&(ptr), sizeof(*(ptr)), true)
should be false in the 2nd case...
/**
* VIR_ALLOC_N:
diff --git a/src/util/virerror.c b/src/util/virerror.c
index c30642a..c033129 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -204,7 +204,7 @@ virLastErrorObject(void)
virErrorPtr err;
err = virThreadLocalGet(&virLastErr);
if (!err) {
- if (VIR_ALLOC(err) < 0)
+ if (VIR_ALLOC_NOOOM(err) < 0)
return NULL;
if (virThreadLocalSet(&virLastErr, err) < 0)
VIR_FREE(err);
ACK with nit fixed.
6:44 a.m.
New subject: [libvirt] [PATCH RFC 2/2] util: Don't report OOM twice
Adapt code under src/util/ to fact, that VIR_ALLOC* now reports OOM
error. There is no need to report it twice now.
---
src/util/iohelper.c | 4 +---
src/util/virauthconfig.c | 8 ++------
src/util/vircommand.c | 13 +++----------
src/util/virconf.c | 10 ++--------
src/util/virdnsmasq.c | 19 +++++++------------
src/util/vireventpoll.c | 4 +---
src/util/virfile.c | 4 +---
src/util/virhash.c | 10 ++--------
src/util/virkeyfile.c | 4 +---
src/util/virlockspace.c | 11 +++--------
src/util/virnetdev.c | 4 +---
src/util/virnetdevbandwidth.c | 12 +++---------
src/util/virnetdevmacvlan.c | 4 ++--
src/util/virnetdevvlan.c | 4 +---
src/util/virnetdevvportprofile.c | 4 +---
src/util/virnetlink.c | 8 ++------
src/util/virobject.c | 9 ++++-----
src/util/virpci.c | 13 +++----------
src/util/virprocess.c | 4 +---
src/util/virsexpr.c | 4 +---
src/util/virstoragefile.c | 24 ++++++------------------
src/util/virstring.c | 5 +++--
src/util/virsysinfo.c | 8 +++-----
src/util/virthreadpool.c | 21 +++++----------------
src/util/virtime.c | 8 ++------
src/util/virtypedparam.c | 36 +++++++++---------------------------
src/util/viruri.c | 2 +-
src/util/virusb.c | 8 ++------
src/util/virutil.c | 36 ++++++++----------------------------
src/util/virxml.c | 1 -
30 files changed, 81 insertions(+), 221 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index 2230bcb..d3c966a 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -94,10 +94,8 @@ runIO(const char *path, int fd, int oflags, unsigned long long length)
}
buf = base;
#else
- if (VIR_ALLOC_N(buf, buflen + alignMask) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(buf, buflen + alignMask) < 0)
goto cleanup;
- }
base = buf;
buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
#endif
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 1d1f084..8847d9e 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -43,10 +43,8 @@ virAuthConfigPtr virAuthConfigNew(const char *path)
{
virAuthConfigPtr auth;
- if (VIR_ALLOC(auth) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(auth) < 0)
goto error;
- }
if (!(auth->path = strdup(path))) {
virReportOOMError();
@@ -73,10 +71,8 @@ virAuthConfigPtr virAuthConfigNewData(const char *path,
{
virAuthConfigPtr auth;
- if (VIR_ALLOC(auth) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(auth) < 0)
goto error;
- }
if (!(auth->path = strdup(path))) {
virReportOOMError();
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index ac56a63..052f1e7 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1804,17 +1804,13 @@ virCommandProcessIO(virCommandPtr cmd)
* results accumulated over a prior run of the same command. */
if (cmd->outbuf) {
outfd = cmd->outfd;
- if (VIR_REALLOC_N(*cmd->outbuf, 1) < 0) {
- virReportOOMError();
+ if (VIR_REALLOC_N(*cmd->outbuf, 1) < 0)
ret = -1;
- }
}
if (cmd->errbuf) {
errfd = cmd->errfd;
- if (VIR_REALLOC_N(*cmd->errbuf, 1) < 0) {
- virReportOOMError();
+ if (VIR_REALLOC_N(*cmd->errbuf, 1) < 0)
ret = -1;
- }
}
if (ret == -1)
goto cleanup;
@@ -1888,10 +1884,8 @@ virCommandProcessIO(virCommandPtr cmd)
else
errfd = -1;
} else {
- if (VIR_REALLOC_N(*buf, *len + done + 1) < 0) {
- virReportOOMError();
+ if (VIR_REALLOC_N(*buf, *len + done + 1) < 0)
goto cleanup;
- }
memcpy(*buf + *len, data, done);
*len += done;
}
@@ -2435,7 +2429,6 @@ int virCommandHandshakeWait(virCommandPtr cmd)
char *msg;
ssize_t len;
if (VIR_ALLOC_N(msg, 1024) < 0) {
- virReportOOMError();
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
return -1;
}
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 16f074a..1c7075f 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -173,10 +173,8 @@ virConfNew(void)
{
virConfPtr ret;
- if (VIR_ALLOC(ret) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(ret) < 0)
return NULL;
- }
ret->filename = NULL;
ret->flags = 0;
@@ -225,10 +223,8 @@ virConfAddEntry(virConfPtr conf, char *name, virConfValuePtr value,
char *comm)
if ((comm == NULL) && (name == NULL))
return NULL;
- if (VIR_ALLOC(ret) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(ret) < 0)
return NULL;
- }
ret->name = name;
ret->value = value;
@@ -538,7 +534,6 @@ virConfParseValue(virConfParserCtxtPtr ctxt)
return NULL;
}
if (VIR_ALLOC(ret) < 0) {
- virReportOOMError();
virConfFreeList(lst);
VIR_FREE(str);
return NULL;
@@ -900,7 +895,6 @@ virConfSetValue(virConfPtr conf,
if (!cur) {
if (VIR_ALLOC(cur) < 0) {
- virReportOOMError();
virConfFreeValue(value);
return -1;
}
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 2e63d83..2e2ccd1 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -117,8 +117,10 @@ addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
if (VIR_ALLOC(addnhostsfile->hosts[idx].hostnames) < 0)
goto alloc_error;
- if (virAsprintf(&addnhostsfile->hosts[idx].ip, "%s", ipstr) <
0)
+ if (virAsprintf(&addnhostsfile->hosts[idx].ip, "%s", ipstr) <
0) {
+ virReportOOMError();
goto alloc_error;
+ }
addnhostsfile->hosts[idx].nhostnames = 0;
addnhostsfile->nhosts++;
@@ -137,7 +139,6 @@ addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
return 0;
alloc_error:
- virReportOOMError();
VIR_FREE(ipstr);
return -1;
}
@@ -148,10 +149,8 @@ addnhostsNew(const char *name,
{
dnsmasqAddnHostsfile *addnhostsfile;
- if (VIR_ALLOC(addnhostsfile) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(addnhostsfile) < 0)
return NULL;
- }
addnhostsfile->hosts = NULL;
addnhostsfile->nhosts = 0;
@@ -308,7 +307,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
{
char *ipstr = NULL;
if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1) < 0)
- goto alloc_error;
+ return -1;
if (!(ipstr = virSocketAddrFormat(ip)))
return -1;
@@ -359,10 +358,8 @@ hostsfileNew(const char *name,
{
dnsmasqHostsfile *hostsfile;
- if (VIR_ALLOC(hostsfile) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(hostsfile) < 0)
return NULL;
- }
hostsfile->hosts = NULL;
hostsfile->nhosts = 0;
@@ -463,10 +460,8 @@ dnsmasqContextNew(const char *network_name,
{
dnsmasqContext *ctx;
- if (VIR_ALLOC(ctx) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(ctx) < 0)
return NULL;
- }
if (!(ctx->config_dir = strdup(config_dir))) {
virReportOOMError();
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index 5c6d31b..95b60d6 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -379,10 +379,8 @@ static struct pollfd *virEventPollMakePollFDs(int *nfds) {
}
/* Setup the poll file handle data structs */
- if (VIR_ALLOC_N(fds, *nfds) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(fds, *nfds) < 0)
return NULL;
- }
*nfds = 0;
for (i = 0 ; i < eventLoop.handlesCount ; i++) {
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 4a9fa81..8442c7b 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -193,10 +193,8 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
return NULL;
}
- if (VIR_ALLOC(ret) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(ret) < 0)
return NULL;
- }
mode = fcntl(*fd, F_GETFL);
diff --git a/src/util/virhash.c b/src/util/virhash.c
index 2fe8751..dc4ab4b 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -126,10 +126,8 @@ virHashTablePtr virHashCreateFull(ssize_t size,
if (size <= 0)
size = 256;
- if (VIR_ALLOC(table) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(table) < 0)
return NULL;
- }
table->seed = virRandomBits(32);
table->size = size;
@@ -141,7 +139,6 @@ virHashTablePtr virHashCreateFull(ssize_t size,
table->keyFree = keyFree;
if (VIR_ALLOC_N(table->table, size) < 0) {
- virReportOOMError();
VIR_FREE(table);
return NULL;
}
@@ -201,7 +198,6 @@ virHashGrow(virHashTablePtr table, size_t size)
return -1;
if (VIR_ALLOC_N(table->table, size) < 0) {
- virReportOOMError();
table->table = oldtable;
return -1;
}
@@ -670,10 +666,8 @@ virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
if (numElems < 0)
return NULL;
- if (VIR_ALLOC_N(iter.sortArray, numElems + 1)) {
- virReportOOMError();
+ if (VIR_ALLOC_N(iter.sortArray, numElems + 1))
return NULL;
- }
virHashForEach(table, virHashGetKeysIterator, &iter);
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index d77e95d..fe2f7e5 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -286,10 +286,8 @@ virKeyFilePtr virKeyFileNew(void)
{
virKeyFilePtr conf;
- if (VIR_ALLOC(conf) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(conf) < 0)
goto error;
- }
if (!(conf->groups = virHashCreate(10,
virKeyFileEntryFree)))
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 4ff0f3a..cdccdda 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -224,7 +224,7 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace,
res->lockHeld = true;
if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0)
- goto no_memory;
+ goto error;
res->owners[res->nOwners-1] = owner;
@@ -349,10 +349,8 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr
object)
size_t j;
int m;
- if (VIR_ALLOC(res) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(res) < 0)
goto error;
- }
res->fd = -1;
if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
@@ -420,7 +418,6 @@ virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr
object)
res->nOwners = m;
if (VIR_ALLOC_N(res->owners, res->nOwners) < 0) {
- virReportOOMError();
virLockSpaceResourceFree(res);
goto error;
}
@@ -649,10 +646,8 @@ int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
if ((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) &&
(flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) {
- if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0) {
- virReportOOMError();
+ if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0)
goto cleanup;
- }
res->owners[res->nOwners-1] = owner;
goto done;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 00e0f94..652668e 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -995,10 +995,8 @@ virNetDevGetVirtualFunctions(const char *pfname,
n_vfname) < 0)
goto cleanup;
- if (VIR_ALLOC_N(*vfname, *n_vfname) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(*vfname, *n_vfname) < 0)
goto cleanup;
- }
for (i = 0; i < *n_vfname; i++)
{
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 2c5b63a..2f4baae 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -296,25 +296,19 @@ virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest,
return 0;
}
- if (VIR_ALLOC(*dest) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(*dest) < 0)
goto cleanup;
- }
if (src->in) {
- if (VIR_ALLOC((*dest)->in) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC((*dest)->in) < 0)
goto cleanup;
- }
memcpy((*dest)->in, src->in, sizeof(*src->in));
}
if (src->out) {
- if (VIR_ALLOC((*dest)->out) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC((*dest)->out) < 0)
VIR_FREE((*dest)->in);
goto cleanup;
- }
memcpy((*dest)->out, src->out, sizeof(*src->out));
}
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index ddea11f..1e351ef 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -775,11 +775,11 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
if (virtPortProfile && virNetlinkEventServiceIsRunning(NETLINK_ROUTE)) {
if (VIR_ALLOC(calld) < 0)
- goto memory_error;
+ goto error;
if ((calld->cr_ifname = strdup(ifname)) == NULL)
goto memory_error;
if (VIR_ALLOC(calld->virtPortProfile) < 0)
- goto memory_error;
+ goto error;
memcpy(calld->virtPortProfile, virtPortProfile, sizeof(*virtPortProfile));
virMacAddrSet(&calld->macaddress, macaddress);
if ((calld->linkdev = strdup(linkdev)) == NULL)
diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c
index 2fe2017..e05e7e6 100644
--- a/src/util/virnetdevvlan.c
+++ b/src/util/virnetdevvlan.c
@@ -82,10 +82,8 @@ virNetDevVlanCopy(virNetDevVlanPtr dst, const virNetDevVlanPtr src)
if (!src || src->nTags == 0)
return 0;
- if (VIR_ALLOC_N(dst->tag, src->nTags) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(dst->tag, src->nTags) < 0)
return -1;
- }
dst->trunk = src->trunk;
dst->nTags = src->nTags;
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index bb97e3a..c51854a 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -409,10 +409,8 @@ int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result,
}
/* at least one of the source profiles is non-empty */
- if (VIR_ALLOC(*result) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(*result) < 0)
return ret;
- }
/* start with the interface's profile. There are no pointers in a
* virtualPortProfile, so a shallow copy is sufficient.
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0b36fdc..204eef3 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -516,10 +516,8 @@ virNetlinkEventServiceStart(unsigned int protocol, unsigned int
groups)
VIR_INFO("starting netlink event service with protocol %d", protocol);
- if (VIR_ALLOC(srv) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(srv) < 0)
return -1;
- }
if (virMutexInit(&srv->lock) < 0) {
VIR_FREE(srv);
@@ -645,10 +643,8 @@ virNetlinkEventAddClient(virNetlinkEventHandleCallback handleCB,
VIR_DEBUG("Used %zu handle slots, adding at least %d more",
srv->handlesAlloc, NETLINK_EVENT_ALLOC_EXTENT);
if (VIR_RESIZE_N(srv->handles, srv->handlesAlloc,
- srv->handlesCount, NETLINK_EVENT_ALLOC_EXTENT) < 0) {
- virReportOOMError();
+ srv->handlesCount, NETLINK_EVENT_ALLOC_EXTENT) < 0)
goto error;
- }
}
r = srv->handlesCount++;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 808edc4..d6a8b3a 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -132,7 +132,7 @@ virClassPtr virClassNew(virClassPtr parent,
}
if (VIR_ALLOC(klass) < 0)
- goto no_memory;
+ goto cleanup;
klass->parent = parent;
if (!(klass->name = strdup(name)))
@@ -144,8 +144,9 @@ virClassPtr virClassNew(virClassPtr parent,
return klass;
no_memory:
- VIR_FREE(klass);
virReportOOMError();
+cleanup:
+ VIR_FREE(klass);
return NULL;
}
@@ -188,10 +189,8 @@ void *virObjectNew(virClassPtr klass)
virObjectPtr obj = NULL;
char *somebytes;
- if (VIR_ALLOC_N(somebytes, klass->objectSize) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(somebytes, klass->objectSize) < 0)
return NULL;
- }
obj = (virObjectPtr)somebytes;
obj->magic = klass->magic;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 4bb82aa..570dfa1 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1374,10 +1374,8 @@ virPCIDeviceNew(unsigned domain,
char *vendor = NULL;
char *product = NULL;
- if (VIR_ALLOC(dev) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(dev) < 0)
return NULL;
- }
dev->domain = domain;
dev->bus = bus;
@@ -1558,10 +1556,8 @@ virPCIDeviceListAdd(virPCIDeviceListPtr list,
return -1;
}
- if (VIR_REALLOC_N(list->devs, list->count+1) < 0) {
- virReportOOMError();
+ if (VIR_REALLOC_N(list->devs, list->count+1) < 0)
return -1;
- }
list->devs[list->count++] = dev;
@@ -1923,10 +1919,8 @@ virPCIGetDeviceAddressFromSysfsLink(const char *device_link,
}
config_address = basename(device_path);
- if (VIR_ALLOC(*bdf) != 0) {
- virReportOOMError();
+ if (VIR_ALLOC(*bdf) != 0)
goto out;
- }
if (virPCIParseDeviceAddress(config_address, *bdf) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2016,7 +2010,6 @@ virPCIGetVirtualFunctions(const char *sysfs_path,
*num_virtual_functions);
if (VIR_REALLOC_N(*virtual_functions,
(*num_virtual_functions) + 1) != 0) {
- virReportOOMError();
VIR_FREE(device_link);
goto out;
}
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index a492bd1..4a81503 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -527,10 +527,8 @@ int virProcessGetNamespaces(pid_t pid,
if (de->d_name[0] == '.')
continue;
- if (VIR_EXPAND_N(*fdlist, *nfdlist, 1) < 0) {
- virReportOOMError();
+ if (VIR_EXPAND_N(*fdlist, *nfdlist, 1) < 0)
goto cleanup;
- }
if (virAsprintf(&nsfile, "%s/%s", nsdir, de->d_name) < 0) {
virReportOOMError();
diff --git a/src/util/virsexpr.c b/src/util/virsexpr.c
index 81dfbe6..5c19da4 100644
--- a/src/util/virsexpr.c
+++ b/src/util/virsexpr.c
@@ -48,10 +48,8 @@ sexpr_new(void)
{
struct sexpr *ret;
- if (VIR_ALLOC(ret) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(ret) < 0)
return NULL;
- }
ret->kind = SEXPR_NIL;
return ret;
}
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index aabb5c8..f12c25e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -318,10 +318,8 @@ qcowXGetBackingStore(char **res,
return BACKING_STORE_INVALID;
if (size + 1 == 0)
return BACKING_STORE_INVALID;
- if (VIR_ALLOC_N(*res, size + 1) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(*res, size + 1) < 0)
return BACKING_STORE_ERROR;
- }
memcpy(*res, buf + offset, size);
(*res)[size] = '\0';
@@ -396,10 +394,8 @@ vmdk4GetBackingStore(char **res,
size_t len;
int ret = BACKING_STORE_ERROR;
- if (VIR_ALLOC_N(desc, STORAGE_MAX_HEAD + 1) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(desc, STORAGE_MAX_HEAD + 1) < 0)
goto cleanup;
- }
*res = NULL;
/*
@@ -481,10 +477,8 @@ qedGetBackingStore(char **res,
return BACKING_STORE_OK;
if (offset + size > buf_size || offset + size < offset)
return BACKING_STORE_INVALID;
- if (VIR_ALLOC_N(*res, size + 1) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(*res, size + 1) < 0)
return BACKING_STORE_ERROR;
- }
memcpy(*res, buf + offset, size);
(*res)[size] = '\0';
@@ -686,10 +680,8 @@ virStorageFileGetMetadataInternal(const char *path,
VIR_DEBUG("path=%s, fd=%d, format=%d", path, fd, format);
- if (VIR_ALLOC(meta) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(meta) < 0)
return NULL;
- }
if (fstat(fd, &sb) < 0) {
virReportSystemError(errno,
@@ -707,10 +699,8 @@ virStorageFileGetMetadataInternal(const char *path,
goto cleanup;
}
- if (VIR_ALLOC_N(buf, len) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(buf, len) < 0)
goto cleanup;
- }
if ((len = read(fd, buf, len)) < 0) {
virReportSystemError(errno, _("cannot read header '%s'"),
path);
@@ -849,10 +839,8 @@ virStorageFileProbeFormatFromFD(const char *path, int fd)
return VIR_STORAGE_FILE_DIR;
}
- if (VIR_ALLOC_N(head, len) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(head, len) < 0)
return -1;
- }
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
virReportSystemError(errno, _("cannot set to start of '%s'"),
path);
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 0420ca3..35d16be 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -79,7 +79,7 @@ char **virStringSplit(const char *string,
size_t len = tmp - remainder;
if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0)
- goto no_memory;
+ goto error;
if (!(tokens[ntokens] = strndup(remainder, len)))
goto no_memory;
@@ -90,7 +90,7 @@ char **virStringSplit(const char *string,
}
if (*string) {
if (VIR_RESIZE_N(tokens, maxtokens, ntokens, 1) < 0)
- goto no_memory;
+ goto error;
if (!(tokens[ntokens] = strdup(remainder)))
goto no_memory;
@@ -105,6 +105,7 @@ char **virStringSplit(const char *string,
no_memory:
virReportOOMError();
+error:
for (i = 0 ; i < ntokens ; i++)
VIR_FREE(tokens[i]);
VIR_FREE(tokens);
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 83c445d..cde7611 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -331,10 +331,8 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret)
while ((tmp_base = strstr(tmp_base, "processor "))
&& (tmp_base = virSysinfoParseLine(tmp_base, "processor ",
&procline))) {
- if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) {
- virReportOOMError();
+ if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0)
goto cleanup;
- }
processor = &ret->processor[ret->nprocessor - 1];
processor->processor_manufacturer = strdup(manufacturer);
if (!virSysinfoParseDelimited(procline, "version",
@@ -778,7 +776,7 @@ virSysinfoRead(void) {
goto cleanup;
if (VIR_ALLOC(ret) < 0)
- goto no_memory;
+ goto error;
ret->type = VIR_SYSINFO_SMBIOS;
@@ -806,7 +804,7 @@ cleanup:
no_memory:
virReportOOMError();
-
+error:
virSysinfoDefFree(ret);
ret = NULL;
goto cleanup;
diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index e657145..b1e2c0c 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -169,10 +169,8 @@ virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
if (minWorkers > maxWorkers)
minWorkers = maxWorkers;
- if (VIR_ALLOC(pool) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(pool) < 0)
return NULL;
- }
pool->jobList.tail = pool->jobList.head = NULL;
@@ -193,10 +191,8 @@ virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
pool->maxWorkers = maxWorkers;
for (i = 0; i < minWorkers; i++) {
- if (VIR_ALLOC(data) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(data) < 0)
goto error;
- }
data->pool = pool;
data->cond = &pool->cond;
@@ -216,10 +212,8 @@ virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
goto error;
for (i = 0; i < prioWorkers; i++) {
- if (VIR_ALLOC(data) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(data) < 0)
goto error;
- }
data->pool = pool;
data->cond = &pool->prioCond;
data->priority = true;
@@ -313,14 +307,11 @@ int virThreadPoolSendJob(virThreadPoolPtr pool,
if (pool->freeWorkers - pool->jobQueueDepth <= 0 &&
pool->nWorkers < pool->maxWorkers) {
- if (VIR_EXPAND_N(pool->workers, pool->nWorkers, 1) < 0) {
- virReportOOMError();
+ if (VIR_EXPAND_N(pool->workers, pool->nWorkers, 1) < 0)
goto error;
- }
if (VIR_ALLOC(data) < 0) {
pool->nWorkers--;
- virReportOOMError();
goto error;
}
@@ -337,10 +328,8 @@ int virThreadPoolSendJob(virThreadPoolPtr pool,
}
}
- if (VIR_ALLOC(job) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(job) < 0)
goto error;
- }
job->data = jobData;
job->priority = priority;
diff --git a/src/util/virtime.c b/src/util/virtime.c
index b54a4e8..4684716 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -304,10 +304,8 @@ char *virTimeStringNow(void)
{
char *ret;
- if (VIR_ALLOC_N(ret, VIR_TIME_STRING_BUFLEN) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(ret, VIR_TIME_STRING_BUFLEN) < 0)
return NULL;
- }
if (virTimeStringNowRaw(ret) < 0) {
virReportSystemError(errno, "%s",
@@ -335,10 +333,8 @@ char *virTimeStringThen(unsigned long long when)
{
char *ret;
- if (VIR_ALLOC_N(ret, VIR_TIME_STRING_BUFLEN) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(ret, VIR_TIME_STRING_BUFLEN) < 0)
return NULL;
- }
if (virTimeStringThenRaw(when, ret) < 0) {
virReportSystemError(errno, "%s",
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index c196321..aa850f5 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -603,10 +603,8 @@ virTypedParamsAddInt(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssign(*params + n, name,
@@ -653,10 +651,8 @@ virTypedParamsAddUInt(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssign(*params + n, name,
@@ -703,10 +699,8 @@ virTypedParamsAddLLong(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssign(*params + n, name,
@@ -753,10 +747,8 @@ virTypedParamsAddULLong(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssign(*params + n, name,
@@ -803,10 +795,8 @@ virTypedParamsAddDouble(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssign(*params + n, name,
@@ -853,10 +843,8 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssign(*params + n, name,
@@ -906,16 +894,12 @@ virTypedParamsAddString(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
- if (value && !(str = strdup(value))) {
- virReportOOMError();
+ if (value && !(str = strdup(value)))
goto error;
- }
if (virTypedParameterAssign(*params + n, name,
VIR_TYPED_PARAM_STRING, str) < 0) {
@@ -968,10 +952,8 @@ virTypedParamsAddFromString(virTypedParameterPtr *params,
virResetLastError();
VIR_TYPED_PARAM_CHECK();
- if (VIR_RESIZE_N(*params, max, n, 1) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(*params, max, n, 1) < 0)
goto error;
- }
*maxparams = max;
if (virTypedParameterAssignFromStr(*params + n, name, type, value) < 0)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 2888cb0..465fdf0 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -168,7 +168,7 @@ virURIParse(const char *uri)
}
if (VIR_ALLOC(ret) < 0)
- goto no_memory;
+ goto error;
if (xmluri->scheme &&
!(ret->scheme = strdup(xmluri->scheme)))
diff --git a/src/util/virusb.c b/src/util/virusb.c
index 5974602..4ba6de5 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -332,10 +332,8 @@ virUSBDeviceNew(unsigned int bus,
{
virUSBDevicePtr dev;
- if (VIR_ALLOC(dev) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC(dev) < 0)
return NULL;
- }
dev->bus = bus;
dev->dev = devno;
@@ -454,10 +452,8 @@ virUSBDeviceListAdd(virUSBDeviceListPtr list,
return -1;
}
- if (VIR_REALLOC_N(list->devs, list->count+1) < 0) {
- virReportOOMError();
+ if (VIR_REALLOC_N(list->devs, list->count+1) < 0)
return -1;
- }
list->devs[list->count++] = dev;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 42b4295..7f375d4 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -368,10 +368,8 @@ virPipeReadUntilEOF(int outfd, int errfd,
buf = ((fds[i].fd == outfd) ? outbuf : errbuf);
size = (*buf ? strlen(*buf) : 0);
- if (VIR_REALLOC_N(*buf, size+got+1) < 0) {
- virReportOOMError();
+ if (VIR_REALLOC_N(*buf, size+got+1) < 0)
goto error;
- }
memmove(*buf+size, data, got);
(*buf)[size+got] = '\0';
}
@@ -2233,10 +2231,8 @@ char *virIndexToDiskName(int idx, const char *prefix)
offset = strlen(prefix);
- if (VIR_ALLOC_N(name, offset + i + 1)) {
- virReportOOMError();
+ if (VIR_ALLOC_N(name, offset + i + 1))
return NULL;
- }
strcpy(name, prefix);
name[offset + i] = '\0';
@@ -2355,10 +2351,8 @@ static char *virGetUserEnt(uid_t uid,
if (val < 0)
strbuflen = 1024;
- if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(strbuf, strbuflen) < 0)
return NULL;
- }
/*
* From the manpage (terrifying but true):
@@ -2369,7 +2363,6 @@ static char *virGetUserEnt(uid_t uid,
*/
while ((rc = getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) {
if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) {
- virReportOOMError();
VIR_FREE(strbuf);
return NULL;
}
@@ -2408,10 +2401,8 @@ static char *virGetGroupEnt(gid_t gid)
if (val < 0)
strbuflen = 1024;
- if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(strbuf, strbuflen) < 0)
return NULL;
- }
/*
* From the manpage (terrifying but true):
@@ -2422,7 +2413,6 @@ static char *virGetGroupEnt(gid_t gid)
*/
while ((rc = getgrgid_r(gid, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) {
if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) {
- virReportOOMError();
VIR_FREE(strbuf);
return NULL;
}
@@ -2527,16 +2517,12 @@ virGetUserIDByName(const char *name, uid_t *uid)
if (val < 0)
strbuflen = 1024;
- if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(strbuf, strbuflen) < 0)
goto cleanup;
- }
while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) {
- if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0)
goto cleanup;
- }
}
if (!pw) {
@@ -2611,16 +2597,12 @@ virGetGroupIDByName(const char *name, gid_t *gid)
if (val < 0)
strbuflen = 1024;
- if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
- virReportOOMError();
+ if (VIR_ALLOC_N(strbuf, strbuflen) < 0)
goto cleanup;
- }
while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) {
- if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) {
- virReportOOMError();
+ if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0)
goto cleanup;
- }
}
if (!gr) {
@@ -2708,14 +2690,12 @@ virSetUIDGID(uid_t uid, gid_t gid)
bufsize = 16384;
if (VIR_ALLOC_N(buf, bufsize) < 0) {
- virReportOOMError();
err = ENOMEM;
goto error;
}
while ((rc = getpwuid_r(uid, &pwd, buf, bufsize,
&pwd_result)) == ERANGE) {
if (VIR_RESIZE_N(buf, bufsize, bufsize, bufsize) < 0) {
- virReportOOMError();
err = ENOMEM;
goto error;
}
diff --git a/src/util/virxml.c b/src/util/virxml.c
index aa55a33..c8ec805 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -609,7 +609,6 @@ virXPathNodeSet(const char *xpath,
ret = obj->nodesetval->nodeNr;
if (list != NULL && ret) {
if (VIR_ALLOC_N(*list, ret) < 0) {
- virReportOOMError();
ret = -1;
} else {
memcpy(*list, obj->nodesetval->nodeTab,
--
1.8.1.5
12:32 p.m.
New subject: [libvirt] [PATCH RFC 2/2] util: Don't report OOM twice
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
Adapt code under src/util/ to fact, that VIR_ALLOC* now reports OOM
error. There is no need to report it twice now.
---
src/util/iohelper.c | 4 +---
src/util/virauthconfig.c | 8 ++------
src/util/vircommand.c | 13 +++----------
src/util/virconf.c | 10 ++--------
src/util/virdnsmasq.c | 19 +++++++------------
src/util/vireventpoll.c | 4 +---
src/util/virfile.c | 4 +---
src/util/virhash.c | 10 ++--------
src/util/virkeyfile.c | 4 +---
src/util/virlockspace.c | 11 +++--------
src/util/virnetdev.c | 4 +---
src/util/virnetdevbandwidth.c | 12 +++---------
src/util/virnetdevmacvlan.c | 4 ++--
src/util/virnetdevvlan.c | 4 +---
src/util/virnetdevvportprofile.c | 4 +---
src/util/virnetlink.c | 8 ++------
src/util/virobject.c | 9 ++++-----
src/util/virpci.c | 13 +++----------
src/util/virprocess.c | 4 +---
src/util/virsexpr.c | 4 +---
src/util/virstoragefile.c | 24 ++++++------------------
src/util/virstring.c | 5 +++--
src/util/virsysinfo.c | 8 +++-----
src/util/virthreadpool.c | 21 +++++----------------
src/util/virtime.c | 8 ++------
src/util/virtypedparam.c | 36 +++++++++---------------------------
src/util/viruri.c | 2 +-
src/util/virusb.c | 8 ++------
src/util/virutil.c | 36 ++++++++----------------------------
src/util/virxml.c | 1 -
30 files changed, 81 insertions(+), 221 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index aa55a33..c8ec805 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -609,7 +609,6 @@ virXPathNodeSet(const char *xpath,
ret = obj->nodesetval->nodeNr;
if (list != NULL && ret) {
if (VIR_ALLOC_N(*list, ret) < 0) {
- virReportOOMError();
ret = -1;
} else {
memcpy(*list, obj->nodesetval->nodeTab,
Maybe an sed script could help here at least in removing the
virReportOOMError, not so easily in removing the '{' and '}'.
Now I doubt we will see lots of OOM errors, but there are a lot more
that may now report twice ...
ACK
12:48 p.m.
New subject: [libvirt] [PATCH RFC 2/2] util: Don't report OOM twice
On 22.03.2013 18:32, Stefan Berger wrote:
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
> Adapt code under src/util/ to fact, that VIR_ALLOC* now reports OOM
> error. There is no need to report it twice now.
> ---
Maybe an sed script could help here at least in removing the
virReportOOMError, not so easily in removing the '{' and '}'.
Now I doubt we will see lots of OOM errors, but there are a lot more
that may now report twice ...
ACK
Yep. This is just a patch to show off how much we are going to save.
But now that I am thinking this over again - maybe virAsprintf deserves
to report OOM error as well (for virlog.c and virerror.c we are gonna
need virAsprintf variant without OOM error reporting). And probably a
wrapper over strdup as well - again with OOM error reporting. I just
don't know if I should do all the adaptation in one patch patch set or
in three different series.
BTW: I am not that strong in RE, so I am doing this with vim macros.
Michal
7:24 a.m.
On Fri, Mar 22, 2013 at 12:44:55PM +0100, Michal Privoznik wrote:
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) < 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC? It would have three nice features:
1) sizeof(code base) gets lower. A lot lower.
Yep, makes sense.
2) even for callers which don't follow the schema described
above, there is no harm reporting so serious error in the
logs. No matter that the callee may fall back and return
success.
I think I'd like some analysis of just how many places we
have which call VIR_ALLOC, but don't want to report errors.
Ought to be possible to grep it, or worst case a quick perl
script to analyse thing
At very least we need to be careful with virerror.c and
virlog.c, since both of those allocate memory, and we
don't want them to cause recursion here.
3) Removing virReportOOMError() from the schema does not need to
be done at once, but can be split into several patches. In the
worst case scenario - the error gets reported twice.
If we do this, we must convert everything before any release.
Overwriting errors is something to be avoided, so I don't
want us to make this worse.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
8:53 p.m.
On 03/22/2013 07:44 AM, Michal Privoznik wrote:
Currently, our code is plenty of following scheme:
if (VIR_ALLOC(dummyPtr) < 0) {
virReportOOMError();
goto cleanup;
}
or something similar. What if we just move the OOM reporting into
VIR_ALLOC?
One thing you might want to consider is the case where the OOM is caused
by some bug in the code, for example incorrectly computing the size of a
buffer to allocate, or a loop that keeps allocating again and again
until failure. Currently we might get some useful information in the
form of the function and line number where the error occurred. With your
change that information would be lost.
Likely the times when this would be helpful would be extremely rare, but
just in case, you might want to think about having the VIR_ALLOC* macros
send __FILE__, __FUNCTION__, and __LINE__ to virAlloc* to be passed on
to a new variation of virReportOOMError() that takes those as args
rather than filling them out inline.
4261
days inactive
4262
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Laine Stump -
Michal Privoznik -
Stefan Berger