From: Simon Marchi Date: Mon, 28 Oct 2019 00:45:18 +0000 (-0400) Subject: ctf, ctf-writer: Fix -Wnull-dereference warnings X-Git-Tag: v2.0.0-rc2~35 X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=198f26cdfd3effd3272eaf238b3d4a437b6621e9 ctf, ctf-writer: Fix -Wnull-dereference warnings When building at the -O2 optimization level with gcc, I get this: /home/simark/src/babeltrace/src/ctf-writer/resolve.c: In function ‘resolve_type’: /home/simark/src/babeltrace/src/ctf-writer/resolve.c:541:7: error: potential null pointer dereference [-Werror=null-dereference] 541 | int cur_index = type_stack_at(ctx->type_stack, | ^~~~~~~~~ gcc sees that type_stack_at can possibly return NULL, but we dereference it unconditionally. All callers of type_stack_at don't check NULL, so that is a good sign that it never happens. From what I understand, if it were to happen it would be a programming error, not a legitimate error. So I think we can simplify this code by making it a precondition of calling type_stack_at that `stack` should not be NULL and `index` should represent a valid element of `stack`. This gets rid of the warning at the same time. There is the same pattern in the ctf plugin code, so fix it there too. Change-Id: I82a7cdf74ea987a597cdf219346ec3b36e8ab200 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/2268 Reviewed-by: Francis Deslauriers --- diff --git a/src/ctf-writer/resolve.c b/src/ctf-writer/resolve.c index 7bcc924c..4636e1c5 100644 --- a/src/ctf-writer/resolve.c +++ b/src/ctf-writer/resolve.c @@ -201,15 +201,10 @@ size_t type_stack_size(type_stack *stack) static struct type_stack_frame *type_stack_peek(type_stack *stack) { - struct type_stack_frame *entry = NULL; + BT_ASSERT(stack); + BT_ASSERT(!type_stack_empty(stack)); - if (!stack || type_stack_empty(stack)) { - goto end; - } - - entry = g_ptr_array_index(stack, stack->len - 1); -end: - return entry; + return g_ptr_array_index(stack, stack->len - 1); } /* @@ -218,19 +213,12 @@ end: * Return value is owned by `stack`. */ static -struct type_stack_frame *type_stack_at(type_stack *stack, - size_t index) +struct type_stack_frame *type_stack_at(type_stack *stack, size_t index) { - struct type_stack_frame *entry = NULL; + BT_ASSERT(stack); + BT_ASSERT(index < stack->len); - if (!stack || index >= stack->len) { - goto end; - } - - entry = g_ptr_array_index(stack, index); - -end: - return entry; + return g_ptr_array_index(stack, index); } /* diff --git a/src/plugins/ctf/common/metadata/ctf-meta-resolve.c b/src/plugins/ctf/common/metadata/ctf-meta-resolve.c index 630f14ed..42c84df1 100644 --- a/src/plugins/ctf/common/metadata/ctf-meta-resolve.c +++ b/src/plugins/ctf/common/metadata/ctf-meta-resolve.c @@ -181,15 +181,10 @@ size_t field_class_stack_size(field_class_stack *stack) static struct field_class_stack_frame *field_class_stack_peek(field_class_stack *stack) { - struct field_class_stack_frame *entry = NULL; + BT_ASSERT(stack); + BT_ASSERT(!field_class_stack_empty(stack)); - if (!stack || field_class_stack_empty(stack)) { - goto end; - } - - entry = g_ptr_array_index(stack, stack->len - 1); -end: - return entry; + return g_ptr_array_index(stack, stack->len - 1); } /* @@ -199,16 +194,10 @@ static struct field_class_stack_frame *field_class_stack_at(field_class_stack *stack, size_t index) { - struct field_class_stack_frame *entry = NULL; + BT_ASSERT(stack); + BT_ASSERT(index < stack->len); - if (!stack || index >= stack->len) { - goto end; - } - - entry = g_ptr_array_index(stack, index); - -end: - return entry; + return g_ptr_array_index(stack, index); } /*