Merged, thanks!

Mathieu

----- Original Message -----
> From: "Jérémie Galarneau" <[email protected]>
> To: [email protected]
> Sent: Thursday, October 10, 2013 12:08:30 PM
> Subject: [lttng-dev] [PATCH babeltrace] Fix: Close traces on context  
> destruction
> 
> bt_trace_handle_destroy is called on destruction of the trace_handle
> hash table's elements. This function only frees the trace handle,
> leaving the input traces open.
> 
> This fix sets remove_trace_handle as the value_destroy_func ensuring
> that the format's close_trace function is called before the trace_handle
> is destroyed.
> 
> Signed-off-by: Jérémie Galarneau <[email protected]>
> ---
>  lib/context.c | 58
>  ++++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 36 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/context.c b/lib/context.c
> index dc77366..5dc4119 100644
> --- a/lib/context.c
> +++ b/lib/context.c
> @@ -34,6 +34,7 @@
>  #include <babeltrace/trace-handle-internal.h>
>  #include <babeltrace/trace-collection.h>
>  #include <babeltrace/format.h>
> +#include <babeltrace/format-internal.h>
>  #include <babeltrace/babeltrace-internal.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -44,6 +45,8 @@
>  
>  #include <glib.h>
>  
> +static void remove_trace_handle(struct bt_trace_handle *handle);
> +
>  struct bt_context *bt_context_create(void)
>  {
>       struct bt_context *ctx;
> @@ -56,7 +59,7 @@ struct bt_context *bt_context_create(void)
>       /* Instanciate the trace handle container */
>       ctx->trace_handles = g_hash_table_new_full(g_direct_hash,
>                               g_direct_equal, NULL,
> -                             (GDestroyNotify) bt_trace_handle_destroy);
> +                             (GDestroyNotify) remove_trace_handle);
>  
>       ctx->current_iterator = NULL;
>       ctx->tc = g_new0(struct trace_collection, 1);
> @@ -155,43 +158,39 @@ end:
>  
>  int bt_context_remove_trace(struct bt_context *ctx, int handle_id)
>  {
> -     struct bt_trace_handle *handle;
>       int ret;
>  
> -     if (!ctx)
> -             return -EINVAL;
> -
> -     handle = g_hash_table_lookup(ctx->trace_handles,
> -             (gpointer) (unsigned long) handle_id);
> -     if (!handle)
> -             return -ENOENT;
> +     if (!ctx) {
> +             ret = -EINVAL;
> +             goto end;
> +     }
>  
> -     /* Remove from containers */
> -     bt_trace_collection_remove(ctx->tc, handle->td);
> -     /* Close the trace */
> -     ret = handle->format->close_trace(handle->td);
> -     if (ret) {
> -             fprintf(stderr, "Error in close_trace callback\n");
> -             return ret;
> +     /*
> +      * Remove the handle. remove_trace_handle will be called
> +      * automatically.
> +      */
> +     if (!g_hash_table_remove(ctx->trace_handles,
> +             (gpointer) (unsigned long) handle_id)) {
> +             ret = -ENOENT;
> +             goto end;
>       }
> -     /* Remove and free the handle */
> -     g_hash_table_remove(ctx->trace_handles,
> -                     (gpointer) (unsigned long) handle_id);
> -     return 0;
> +end:
> +     return ret;
>  }
>  
>  static
>  void bt_context_destroy(struct bt_context *ctx)
>  {
>       assert(ctx);
> -     bt_finalize_trace_collection(ctx->tc);
>  
>       /*
>        * Remove all traces. The g_hash_table_destroy will call
> -      * bt_trace_handle_destroy on each elements.
> +      * remove_trace_handle on each element.
>        */
>       g_hash_table_destroy(ctx->trace_handles);
>  
> +     bt_finalize_trace_collection(ctx->tc);
> +
>       /* ctx->tc should always be valid */
>       assert(ctx->tc != NULL);
>       g_free(ctx->tc);
> @@ -211,3 +210,18 @@ void bt_context_put(struct bt_context *ctx)
>       if (ctx->refcount == 0)
>               bt_context_destroy(ctx);
>  }
> +
> +void remove_trace_handle(struct bt_trace_handle *handle)
> +{
> +     int ret;
> +
> +     /* Remove from containers */
> +     bt_trace_collection_remove(handle->td->ctx->tc, handle->td);
> +     /* Close the trace */
> +     ret = handle->format->close_trace(handle->td);
> +     if (ret) {
> +             fprintf(stderr, "Error in close_trace callback\n");
> +     }
> +
> +     bt_trace_handle_destroy(handle);
> +}
> --
> 1.8.4
> 
> 
> _______________________________________________
> lttng-dev mailing list
> [email protected]
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to