Skip to content

Commit

Permalink
applied suggestions from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshuaMoelans committed Oct 31, 2024
1 parent 751e8ff commit 190a4a5
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 8 deletions.
1 change: 0 additions & 1 deletion src/sentry_scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 24 additions & 1 deletion src/sentry_value.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]);
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 4 additions & 6 deletions src/sentry_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down

0 comments on commit 190a4a5

Please sign in to comment.