|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-unstable] libxl: Crash (more sensibly) on malloc failure
# HG changeset patch
# User Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
# Date 1334150053 -3600
# Node ID 26f72d923cb90fad0dd212348f5f8b85588780b0
# Parent 0ff2b4d05c2eaf7ab4cf62ec87f1ceea24f8830c
libxl: Crash (more sensibly) on malloc failure
Formally change the libxl memory allocation failure policy to "crash".
Previously we had a very uneven approach; much code assumed that
libxl__sprintf (for example) would never return NULL, but some code
was written more carefully.
We think it is unlikely that we will be able to make the library
actually robust against allocation failure (since that would be an
awful lot of never-tested error paths) and few calling environments
will be able to cope anyway. So, instead, adopt the alternative
approach: provide allocation functions which never return null, but
will crash the whole process instead.
Consequently,
- New noreturn function libxl__alloc_failed which may be used for
printing a vaguely-useful error message, rather than simply
dereferencing a null pointer.
- libxl__ptr_add now returns void as it crashes on failure.
- libxl__zalloc, _calloc, _strdup, _strndup, crash on failure using
libxl__alloc_failed. So all the code that uses these can no longer
dereference null on malloc failure.
While we're at it, make libxl__ptr_add use realloc rather than
emulating it with calloc and free, and make it grow the array
exponentially rather than linearly.
Things left to do:
- Remove a lot of now-spurious error handling.
- Remove the ERROR_NOMEM error code.
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Committed-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
diff -r 0ff2b4d05c2e -r 26f72d923cb9 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Wed Apr 11 14:14:13 2012 +0100
+++ b/tools/libxl/libxl.h Wed Apr 11 14:14:13 2012 +0100
@@ -194,6 +194,10 @@
* No temporary objects allocated from the pool may be explicitly freed.
* Therefore public functions which initialize a libxl__gc MUST call
* libxl__free_all() before returning.
+ *
+ * Memory allocation failures are not handled gracefully. If malloc
+ * (or realloc) fails, libxl will cause the entire process to print
+ * a message to stderr and exit with status 255.
*/
/*
* libxl types
diff -r 0ff2b4d05c2e -r 26f72d923cb9 tools/libxl/libxl_internal.c
--- a/tools/libxl/libxl_internal.c Wed Apr 11 14:14:13 2012 +0100
+++ b/tools/libxl/libxl_internal.c Wed Apr 11 14:14:13 2012 +0100
@@ -17,40 +17,45 @@
#include "libxl_internal.h"
-int libxl__error_set(libxl__gc *gc, int code)
-{
- return 0;
+void libxl__alloc_failed(libxl_ctx *ctx, const char *func,
+ size_t nmemb, size_t size) {
+#define M "libxl: FATAL ERROR: memory allocation failure"
+#define L (size ? M " (%s, %lu x %lu)\n" : M " (%s)\n"), \
+ func, (unsigned long)nmemb, (unsigned long)size
+ libxl__log(ctx, XTL_CRITICAL, ENOMEM, 0,0, func, L);
+ fprintf(stderr, L);
+ fflush(stderr);
+ _exit(-1);
+#undef M
+#undef L
}
-int libxl__ptr_add(libxl__gc *gc, void *ptr)
+void libxl__ptr_add(libxl__gc *gc, void *ptr)
{
int i;
- void **re;
if (!ptr)
- return 0;
+ return;
/* fast case: we have space in the array for storing the pointer */
for (i = 0; i < gc->alloc_maxsize; i++) {
if (!gc->alloc_ptrs[i]) {
gc->alloc_ptrs[i] = ptr;
- return 0;
+ return;
}
}
- /* realloc alloc_ptrs manually with calloc/free/replace */
- re = calloc(gc->alloc_maxsize + 25, sizeof(void *));
- if (!re)
- return -1;
- for (i = 0; i < gc->alloc_maxsize; i++)
- re[i] = gc->alloc_ptrs[i];
- /* assign the next pointer */
- re[i] = ptr;
+ int new_maxsize = gc->alloc_maxsize * 2 + 25;
+ assert(new_maxsize < INT_MAX / sizeof(void*) / 2);
+ gc->alloc_ptrs = realloc(gc->alloc_ptrs, new_maxsize * sizeof(void *));
+ if (!gc->alloc_ptrs)
+ libxl__alloc_failed(CTX, __func__, new_maxsize, sizeof(void*));
- /* replace the old alloc_ptr */
- free(gc->alloc_ptrs);
- gc->alloc_ptrs = re;
- gc->alloc_maxsize += 25;
- return 0;
+ gc->alloc_ptrs[gc->alloc_maxsize++] = ptr;
+
+ while (gc->alloc_maxsize < new_maxsize)
+ gc->alloc_ptrs[gc->alloc_maxsize++] = 0;
+
+ return;
}
void libxl__free_all(libxl__gc *gc)
@@ -71,10 +76,7 @@ void libxl__free_all(libxl__gc *gc)
void *libxl__zalloc(libxl__gc *gc, int bytes)
{
void *ptr = calloc(bytes, 1);
- if (!ptr) {
- libxl__error_set(gc, ENOMEM);
- return NULL;
- }
+ if (!ptr) libxl__alloc_failed(CTX, __func__, bytes, 1);
libxl__ptr_add(gc, ptr);
return ptr;
@@ -83,10 +85,7 @@ void *libxl__zalloc(libxl__gc *gc, int b
void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size)
{
void *ptr = calloc(nmemb, size);
- if (!ptr) {
- libxl__error_set(gc, ENOMEM);
- return NULL;
- }
+ if (!ptr) libxl__alloc_failed(CTX, __func__, nmemb, size);
libxl__ptr_add(gc, ptr);
return ptr;
@@ -97,9 +96,8 @@ void *libxl__realloc(libxl__gc *gc, void
void *new_ptr = realloc(ptr, new_size);
int i = 0;
- if (new_ptr == NULL && new_size != 0) {
- return NULL;
- }
+ if (new_ptr == NULL && new_size != 0)
+ libxl__alloc_failed(CTX, __func__, new_size, 1);
if (ptr == NULL) {
libxl__ptr_add(gc, new_ptr);
@@ -112,7 +110,6 @@ void *libxl__realloc(libxl__gc *gc, void
}
}
-
return new_ptr;
}
@@ -126,16 +123,13 @@ char *libxl__sprintf(libxl__gc *gc, cons
ret = vsnprintf(NULL, 0, fmt, ap);
va_end(ap);
- if (ret < 0) {
- return NULL;
- }
+ assert(ret >= 0);
s = libxl__zalloc(gc, ret + 1);
- if (s) {
- va_start(ap, fmt);
- ret = vsnprintf(s, ret + 1, fmt, ap);
- va_end(ap);
- }
+ va_start(ap, fmt);
+ ret = vsnprintf(s, ret + 1, fmt, ap);
+ va_end(ap);
+
return s;
}
@@ -143,8 +137,9 @@ char *libxl__strdup(libxl__gc *gc, const
{
char *s = strdup(c);
- if (s)
- libxl__ptr_add(gc, s);
+ if (!s) libxl__alloc_failed(CTX, __func__, strlen(c), 1);
+
+ libxl__ptr_add(gc, s);
return s;
}
@@ -153,8 +148,7 @@ char *libxl__strndup(libxl__gc *gc, cons
{
char *s = strndup(c, n);
- if (s)
- libxl__ptr_add(gc, s);
+ if (!s) libxl__alloc_failed(CTX, __func__, n, 1);
return s;
}
@@ -175,6 +169,9 @@ void libxl__logv(libxl_ctx *ctx, xentool
const char *file, int line, const char *func,
const char *fmt, va_list ap)
{
+ /* WARNING this function may not call any libxl-provided
+ * memory allocation function, as those may
+ * call libxl__alloc_failed which calls libxl__logv. */
char *enomem = "[out of memory formatting log message]";
char *base = NULL;
int rc, esave;
diff -r 0ff2b4d05c2e -r 26f72d923cb9 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Wed Apr 11 14:14:13 2012 +0100
+++ b/tools/libxl/libxl_internal.h Wed Apr 11 14:14:13 2012 +0100
@@ -116,6 +116,12 @@ typedef struct libxl__gc libxl__gc;
typedef struct libxl__egc libxl__egc;
typedef struct libxl__ao libxl__ao;
+void libxl__alloc_failed(libxl_ctx *, const char *func,
+ size_t nmemb, size_t size) __attribute__((noreturn));
+ /* func, size and nmemb are used only in the log message.
+ * You may pass size==0 if size and nmemb are not meaningful
+ * and should not be printed. */
+
typedef struct libxl__ev_fd libxl__ev_fd;
typedef void libxl__ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev,
int fd, short events, short revents);
@@ -380,7 +386,7 @@ static inline libxl_ctx *libxl__gc_owner
* collection on exit from the outermost libxl callframe.
*/
/* register @ptr in @gc for free on exit from outermost libxl callframe. */
-_hidden int libxl__ptr_add(libxl__gc *gc, void *ptr);
+_hidden void libxl__ptr_add(libxl__gc *gc, void *ptr);
/* if this is the outermost libxl callframe then free all pointers in @gc */
_hidden void libxl__free_all(libxl__gc *gc);
/* allocate and zero @bytes. (similar to a gc'd malloc(3)+memzero()) */
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |