ctf, ctf-writer: Fix -Wnull-dereference warnings
authorSimon Marchi <simon.marchi@efficios.com>
Mon, 28 Oct 2019 00:45:18 +0000 (20:45 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Tue, 29 Oct 2019 15:35:18 +0000 (11:35 -0400)
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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2268
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
src/ctf-writer/resolve.c
src/plugins/ctf/common/metadata/ctf-meta-resolve.c

index 7bcc924c4024e8510ce287d264c7dbe3e7e158eb..4636e1c55348786ec6122624ba5cc94244606980 100644 (file)
@@ -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);
 }
 
 /*
index 630f14ed5e780ab11100c346f6df187260fa9d15..42c84df110d9fd7342a815872b129572fcdbfb08 100644 (file)
@@ -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);
 }
 
 /*
This page took 0.027442 seconds and 4 git commands to generate.