From 190a4a5d1dc195d5b038634f83902f79c8e84b0b Mon Sep 17 00:00:00 2001 From: JoshuaMoelans <60878493+JoshuaMoelans@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:04:51 +0100 Subject: [PATCH] applied suggestions from code review --- src/sentry_scope.c | 1 - src/sentry_value.c | 25 ++++++++++++++++++++++++- src/sentry_value.h | 10 ++++------ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 7a2f20a3c..1e7f15e06 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -74,7 +74,6 @@ get_scope(void) g_scope.contexts = sentry_value_new_object(); sentry_value_set_by_key(g_scope.contexts, "os", sentry__get_os_context()); g_scope.breadcrumbs = sentry_value_new_list(); - sentry_value_append(g_scope.breadcrumbs, sentry_value_new_int32(1)); g_scope.level = SENTRY_LEVEL_ERROR; g_scope.client_sdk = get_client_sdk(); g_scope.transaction_object = NULL; diff --git a/src/sentry_value.c b/src/sentry_value.c index 983ae9995..4bfffbc30 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -629,6 +629,19 @@ sentry__value_clone(sentry_value_t value) } } +/** + * This appends `v` to the List `value`. + * To make this work properly as a ring buffer, the value list needs to have + * the ring buffer start index as the first element + * (e.g, 1 until max is exceeded, then it will update for each added item) + * + * It will remove the oldest value in the list, in case the total number of + * items would exceed `max`. + * + * The list is of size `max + 1` to store the start index. + * + * Returns 0 on success. + */ int sentry__value_append_ringbuffer( sentry_value_t value, sentry_value_t v, size_t max) @@ -639,10 +652,17 @@ sentry__value_append_ringbuffer( } list_t *l = thing->payload._ptr; - + if (l->len == 0) { + sentry_value_append(value, sentry_value_new_int32(1)); + } if (l->len < max + 1) { return sentry_value_append(value, v); } + if (l->len > max + 1) { + SENTRY_WARNF("Cannot reduce Ringbuffer list size from %d to %d.", + l->len - 1, max); + goto fail; + } const int32_t start_idx = sentry_value_as_int32(l->items[0]); sentry_value_decref(l->items[start_idx]); @@ -835,6 +855,9 @@ sentry__value_ring_buffer_to_list(const sentry_value_t rb) return sentry_value_new_null(); } const list_t *rb_list = thing->payload._ptr; + if (rb_list->len == 0) { + return sentry_value_new_list(); + } const size_t start_idx = sentry_value_as_int32(rb_list->items[0]); sentry_value_t rv = sentry_value_new_list(); diff --git a/src/sentry_value.h b/src/sentry_value.h index c65b0a221..9cfaddd86 100644 --- a/src/sentry_value.h +++ b/src/sentry_value.h @@ -81,14 +81,12 @@ sentry_value_t sentry__value_clone(sentry_value_t value); /** * This appends `v` to the List `value`. - * To make this work properly as a ring buffer, the value list needs to have - * the ring buffer start index as the first element - * (e.g, 1 until max is exceeded, then it will update for each added item) * - * It will remove the oldest value in the list, in case the total number of - * items would exceed `max`. + * On non-full lists, exponentially reallocate space to accommodate new values + * (until we reach `max`). After reaching max, the oldest value is removed to + * make space for the new one. * - * The list is of size `max + 1` to store the start index. + * `max` should stay fixed over multiple invocations. * * Returns 0 on success. */