[lttng-dev] [PATCH babeltrace 1.3/1.4 v2] Fix: segmentation fault with multi-trace having non-correlated clocks

[lttng-dev] [PATCH babeltrace 1.3/1.4 v2] Fix: segmentation fault with multi-trace having non-correlated clocks

[Mathieu Desnoyers]

This bugfix is extracted from commit

commit 61cf588beae752e5ddfc60b6b5310f769ac9e852
Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Date:   Tue Feb 9 13:27:24 2016 -0500

    Handle negative time and offset from Epoch

Which was fixing this issue, amongst other things.

[ This fix should be applied to Babeltrace stable 1.3 and 1.4 only. ]

[ This v2 patch removes error checking that was introduced in v1. Since
  the callees don't distinguish between -ENOENT and -EINVAL errors,
  we don't know if we need to error out or not. Keep the original
  1.x behavior (no error checking). ]

Fixes: #790

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
---
 lib/context.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/lib/context.c b/lib/context.c
index 45aab34..87901b3 100644
--- a/lib/context.c
+++ b/lib/context.c
@@ -115,7 +115,7 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
 		fprintf(stderr, "[error] [Context] Creating trace handle %s .\n\n",
 				path);
 		ret = -1;
-		goto error;
+		goto error_close;
 	}
 	handle->format = fmt;
 	handle->td = td;
@@ -124,23 +124,19 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
 		handle->path[PATH_MAX - 1] = '\0';
 	}
 
+	ret = bt_trace_collection_add(ctx->tc, td);
+	if (ret != 0)
+		goto error_destroy_handle;
+
 	if (fmt->set_handle)
 		fmt->set_handle(td, handle);
 	if (fmt->set_context)
 		fmt->set_context(td, ctx);
 
-	/* Add new handle to container */
-	g_hash_table_insert(ctx->trace_handles,
-		(gpointer) (unsigned long) handle->id,
-		handle);
-	ret = bt_trace_collection_add(ctx->tc, td);
-	if (ret != 0)
-		goto error;
-
 	if (fmt->convert_index_timestamp) {
 		ret = fmt->convert_index_timestamp(td);
 		if (ret < 0)
-			goto error;
+			goto error_collection_del;
 	}
 
 	if (fmt->timestamp_begin)
@@ -156,9 +152,19 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
 		handle->cycles_timestamp_end = fmt->timestamp_end(td, handle,
 				BT_CLOCK_CYCLES);
 
+	/* Add new handle to container */
+	g_hash_table_insert(ctx->trace_handles,
+		(gpointer) (unsigned long) handle->id,
+		handle);
+
 	return handle->id;
 
-error:
+error_collection_del:
+	/* Remove from containers */
+	bt_trace_collection_remove(handle->td->ctx->tc, handle->td);
+error_destroy_handle:
+	bt_trace_handle_destroy(handle);
+error_close:
 	closeret = fmt->close_trace(td);
 	if (closeret) {
 		fprintf(stderr, "Error in close_trace callback\n");
-- 
2.1.4

[lttng-dev] [PATCH babeltrace 1.3/1.4 v2] Fix: segmentation fault with multi-trace having non-correlated clocks

[Jérémie Galarneau]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3377 bytes --]

Merged in stable-1.3 and stable-1.4. Thanks!

Jérémie

On Thu, May 19, 2016 at 6:24 PM, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
> This bugfix is extracted from commit
>
> commit 61cf588beae752e5ddfc60b6b5310f769ac9e852
> Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> Date:   Tue Feb 9 13:27:24 2016 -0500
>
>     Handle negative time and offset from Epoch
>
> Which was fixing this issue, amongst other things.
>
> [ This fix should be applied to Babeltrace stable 1.3 and 1.4 only. ]
>
> [ This v2 patch removes error checking that was introduced in v1. Since
>   the callees don't distinguish between -ENOENT and -EINVAL errors,
>   we don't know if we need to error out or not. Keep the original
>   1.x behavior (no error checking). ]
>
> Fixes: #790
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  lib/context.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/lib/context.c b/lib/context.c
> index 45aab34..87901b3 100644
> --- a/lib/context.c
> +++ b/lib/context.c
> @@ -115,7 +115,7 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
>                 fprintf(stderr, "[error] [Context] Creating trace handle %s .\n\n",
>                                 path);
>                 ret = -1;
> -               goto error;
> +               goto error_close;
>         }
>         handle->format = fmt;
>         handle->td = td;
> @@ -124,23 +124,19 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
>                 handle->path[PATH_MAX - 1] = '\0';
>         }
>
> +       ret = bt_trace_collection_add(ctx->tc, td);
> +       if (ret != 0)
> +               goto error_destroy_handle;
> +
>         if (fmt->set_handle)
>                 fmt->set_handle(td, handle);
>         if (fmt->set_context)
>                 fmt->set_context(td, ctx);
>
> -       /* Add new handle to container */
> -       g_hash_table_insert(ctx->trace_handles,
> -               (gpointer) (unsigned long) handle->id,
> -               handle);
> -       ret = bt_trace_collection_add(ctx->tc, td);
> -       if (ret != 0)
> -               goto error;
> -
>         if (fmt->convert_index_timestamp) {
>                 ret = fmt->convert_index_timestamp(td);
>                 if (ret < 0)
> -                       goto error;
> +                       goto error_collection_del;
>         }
>
>         if (fmt->timestamp_begin)
> @@ -156,9 +152,19 @@ int bt_context_add_trace(struct bt_context *ctx, const char *path,
>                 handle->cycles_timestamp_end = fmt->timestamp_end(td, handle,
>                                 BT_CLOCK_CYCLES);
>
> +       /* Add new handle to container */
> +       g_hash_table_insert(ctx->trace_handles,
> +               (gpointer) (unsigned long) handle->id,
> +               handle);
> +
>         return handle->id;
>
> -error:
> +error_collection_del:
> +       /* Remove from containers */
> +       bt_trace_collection_remove(handle->td->ctx->tc, handle->td);
> +error_destroy_handle:
> +       bt_trace_handle_destroy(handle);
> +error_close:
>         closeret = fmt->close_trace(td);
>         if (closeret) {
>                 fprintf(stderr, "Error in close_trace callback\n");
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com