From b0a5c4f7009f77d54a8e9bfee300c69218602427 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 Jul 2020 17:24:50 +0100 Subject: [PATCH] libsysprof-capture: Use malloc() rather than g_new0() and friends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Another step away from GLib. This changes the OOM behaviour of the library — previously it would immediately `abort()` on OOM. However, it seems likely that given the small number of allocations libsysprof-capture does, it should be able to recover from an OOM situation more gracefully than larger libraries can — so the new implementation tries to do that. Signed-off-by: Philip Withnall Helps: #40 --- src/libsysprof-capture/mapped-ring-buffer.c | 14 ++++++- .../sysprof-capture-condition.c | 8 +++- .../sysprof-capture-cursor.c | 8 +++- .../sysprof-capture-reader.c | 38 ++++++++++++++++--- .../sysprof-capture-util-private.h | 12 ++++++ .../sysprof-capture-writer.c | 16 ++++++-- src/libsysprof-capture/sysprof-collector.c | 8 +++- 7 files changed, 86 insertions(+), 18 deletions(-) diff --git a/src/libsysprof-capture/mapped-ring-buffer.c b/src/libsysprof-capture/mapped-ring-buffer.c index 1c170c61..81a37f95 100644 --- a/src/libsysprof-capture/mapped-ring-buffer.c +++ b/src/libsysprof-capture/mapped-ring-buffer.c @@ -203,7 +203,10 @@ mapped_ring_buffer_new_reader (size_t buffer_size) header->offset = page_size; header->size = buffer_size - page_size; - self = g_slice_new0 (MappedRingBuffer); + self = sysprof_malloc0 (sizeof (MappedRingBuffer)); + if (self == NULL) + return NULL; + self->ref_count = 1; self->mode = MODE_READER; self->body_size = buffer_size - page_size; @@ -301,7 +304,14 @@ mapped_ring_buffer_new_writer (int fd) return NULL; } - self = g_slice_new0 (MappedRingBuffer); + self = sysprof_malloc0 (sizeof (MappedRingBuffer)); + if (self == NULL) + { + munmap (map, page_size + ((buffer_size - page_size) * 2)); + close (fd); + return NULL; + } + self->ref_count = 1; self->mode = MODE_WRITER; self->fd = fd; diff --git a/src/libsysprof-capture/sysprof-capture-condition.c b/src/libsysprof-capture/sysprof-capture-condition.c index 99219060..1755e3db 100644 --- a/src/libsysprof-capture/sysprof-capture-condition.c +++ b/src/libsysprof-capture/sysprof-capture-condition.c @@ -63,6 +63,7 @@ #include #include "sysprof-capture-condition.h" +#include "sysprof-capture-util-private.h" #include "sysprof-macros-internal.h" /** @@ -204,7 +205,10 @@ sysprof_capture_condition_init (void) { SysprofCaptureCondition *self; - self = g_slice_new0 (SysprofCaptureCondition); + self = sysprof_malloc0 (sizeof (SysprofCaptureCondition)); + if (self == NULL) + return NULL; + self->ref_count = 1; return g_steal_pointer (&self); @@ -294,7 +298,7 @@ sysprof_capture_condition_finalize (SysprofCaptureCondition *self) break; } - g_slice_free (SysprofCaptureCondition, self); + free (self); } SysprofCaptureCondition * diff --git a/src/libsysprof-capture/sysprof-capture-cursor.c b/src/libsysprof-capture/sysprof-capture-cursor.c index a6af3845..5a81d790 100644 --- a/src/libsysprof-capture/sysprof-capture-cursor.c +++ b/src/libsysprof-capture/sysprof-capture-cursor.c @@ -63,6 +63,7 @@ #include "sysprof-capture-condition.h" #include "sysprof-capture-cursor.h" #include "sysprof-capture-reader.h" +#include "sysprof-capture-util-private.h" #define READ_DELEGATE(f) ((ReadDelegate)(f)) @@ -81,7 +82,7 @@ sysprof_capture_cursor_finalize (SysprofCaptureCursor *self) { g_clear_pointer (&self->conditions, g_ptr_array_unref); g_clear_pointer (&self->reader, sysprof_capture_reader_unref); - g_slice_free (SysprofCaptureCursor, self); + free (self); } static SysprofCaptureCursor * @@ -89,7 +90,10 @@ sysprof_capture_cursor_init (void) { SysprofCaptureCursor *self; - self = g_slice_new0 (SysprofCaptureCursor); + self = sysprof_malloc0 (sizeof (SysprofCaptureCursor)); + if (self == NULL) + return NULL; + self->conditions = g_ptr_array_new_with_free_func ((GDestroyNotify) sysprof_capture_condition_unref); self->ref_count = 1; diff --git a/src/libsysprof-capture/sysprof-capture-reader.c b/src/libsysprof-capture/sysprof-capture-reader.c index 6c4862e9..3553efc0 100644 --- a/src/libsysprof-capture/sysprof-capture-reader.c +++ b/src/libsysprof-capture/sysprof-capture-reader.c @@ -126,9 +126,9 @@ sysprof_capture_reader_finalize (SysprofCaptureReader *self) if (self != NULL) { close (self->fd); - g_free (self->buf); + free (self->buf); g_free (self->filename); - g_free (self); + free (self); } } @@ -216,10 +216,23 @@ sysprof_capture_reader_new_from_fd (int fd, assert (fd > -1); - self = g_new0 (SysprofCaptureReader, 1); + self = sysprof_malloc0 (sizeof (SysprofCaptureReader)); + if (self == NULL) + { + g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_NOMEM, "No memory"); + return NULL; + } + self->ref_count = 1; self->bufsz = USHRT_MAX * 2; - self->buf = g_malloc (self->bufsz); + self->buf = sysprof_malloc0 (self->bufsz); + if (self->buf == NULL) + { + free (self); + g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_NOMEM, "No memory"); + return NULL; + } + self->len = 0; self->pos = 0; self->fd = fd; @@ -1156,7 +1169,12 @@ sysprof_capture_reader_copy (SysprofCaptureReader *self) if (-1 == (fd = dup (self->fd))) return NULL; - copy = g_new0 (SysprofCaptureReader, 1); + copy = sysprof_malloc0 (sizeof (SysprofCaptureReader)); + if (copy == NULL) + { + close (fd); + return NULL; + } *copy = *self; @@ -1167,7 +1185,15 @@ sysprof_capture_reader_copy (SysprofCaptureReader *self) copy->st_buf = self->st_buf; copy->st_buf_set = self->st_buf_set; - copy->buf = g_malloc (self->bufsz); + copy->buf = malloc (self->bufsz); + if (copy->buf == NULL) + { + close (fd); + g_free (copy->filename); + free (copy); + return NULL; + } + memcpy (copy->buf, self->buf, self->bufsz); return copy; diff --git a/src/libsysprof-capture/sysprof-capture-util-private.h b/src/libsysprof-capture/sysprof-capture-util-private.h index 44c7d0dc..47c59d13 100644 --- a/src/libsysprof-capture/sysprof-capture-util-private.h +++ b/src/libsysprof-capture/sysprof-capture-util-private.h @@ -62,8 +62,20 @@ # include #endif +#include +#include #include +static inline void * +sysprof_malloc0 (size_t size) +{ + void *ptr = malloc (size); + if (ptr == NULL) + return NULL; + memset (ptr, 0, size); + return ptr; +} + #ifdef __linux__ # define _sysprof_getpagesize() getpagesize() # define _sysprof_pread(a,b,c,d) pread(a,b,c,d) diff --git a/src/libsysprof-capture/sysprof-capture-writer.c b/src/libsysprof-capture/sysprof-capture-writer.c index 23f00243..e74e6c61 100644 --- a/src/libsysprof-capture/sysprof-capture-writer.c +++ b/src/libsysprof-capture/sysprof-capture-writer.c @@ -181,8 +181,8 @@ sysprof_capture_writer_finalize (SysprofCaptureWriter *self) self->fd = -1; } - g_free (self->buf); - g_free (self); + free (self->buf); + free (self); } } @@ -483,10 +483,18 @@ sysprof_capture_writer_new_from_fd (int fd, /* This is only useful on files, memfd, etc */ if (ftruncate (fd, 0) != 0) { /* Do Nothing */ } - self = g_new0 (SysprofCaptureWriter, 1); + self = sysprof_malloc0 (sizeof (SysprofCaptureWriter)); + if (self == NULL) + return NULL; + self->ref_count = 1; self->fd = fd; - self->buf = (guint8 *)g_malloc0 (buffer_size); + self->buf = (uint8_t *) sysprof_malloc0 (buffer_size); + if (self->buf == NULL) + { + free (self); + return NULL; + } self->len = buffer_size; self->next_counter_id = 1; diff --git a/src/libsysprof-capture/sysprof-collector.c b/src/libsysprof-capture/sysprof-collector.c index db177567..482d5362 100644 --- a/src/libsysprof-capture/sysprof-collector.c +++ b/src/libsysprof-capture/sysprof-collector.c @@ -75,6 +75,7 @@ #include "mapped-ring-buffer.h" +#include "sysprof-capture-util-private.h" #include "sysprof-collector.h" #define MAX_UNWIND_DEPTH 128 @@ -214,7 +215,7 @@ sysprof_collector_free (void *data) mapped_ring_buffer_unref (buffer); } - g_free (collector); + free (collector); } } @@ -238,9 +239,12 @@ sysprof_collector_get (void) g_private_replace (&collector_key, COLLECTOR_INVALID); + self = sysprof_malloc0 (sizeof (SysprofCollector)); + if (self == NULL) + return COLLECTOR_INVALID; + G_LOCK (control_fd); - self = g_new0 (SysprofCollector, 1); self->pid = getpid (); #ifdef __linux__ self->tid = syscall (__NR_gettid, 0);