[vlc-devel] [PATCH] libvlc: add internal function libvlc_get_thread_context()

Rémi Denis-Courmont remi at remlab.net
Thu Feb 27 19:27:24 CET 2014


Le jeudi 27 février 2014, 19:07:44 Jerome Forissier a écrit :
> This commit rewrites the error handling functions (libvlc_errmsg() et al.)
> so that they use a common per-thread structure. Should other parts of the
> library need to store per-thread data in the future, no additional
> vlc_threadvar_t will be required.
> ---
>  lib/Makefile.am       |  1 +
>  lib/context.c         | 64
> +++++++++++++++++++++++++++++++++++++++++++++++++++ lib/error.c           |
> 40 +++++++++++---------------------
>  lib/libvlc_internal.h |  8 +++++++
>  4 files changed, 86 insertions(+), 27 deletions(-)
>  create mode 100644 lib/context.c
> 
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 0e38646..6f782d3 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -36,6 +36,7 @@ libvlc_la_SOURCES = \
>  	media_internal.h \
>  	media_list_internal.h \
>  	media_player_internal.h \
> +	context.c \
>  	core.c \
>  	error.c \
>  	log.c \
> diff --git a/lib/context.c b/lib/context.c
> new file mode 100644
> index 0000000..a14541c
> --- /dev/null
> +++ b/lib/context.c
> @@ -0,0 +1,64 @@
> +/**************************************************************************
> *** + * context.c: Hold per-thread data for libvlc
> +
> ***************************************************************************
> ** + * Copyright (C) 2014 VLC authors and VideoLAN
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or + *
> (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA. +
> ***************************************************************************
> **/ +
> +#include "libvlc_internal.h"
> +
> +#include <assert.h>
> +#include <vlc/libvlc.h>
> +
> +
> +static vlc_threadvar_t context;
> +
> +static vlc_mutex_t lock = VLC_STATIC_MUTEX;
> +static uintptr_t refs = 0;
> +
> +static void free_context(void *ptr)
> +{
> +    libvlc_thread_context_t *ctx = (libvlc_thread_context_t *)ptr;
> +    if (ctx->err.free)
> +        ctx->err.free(ctx->err.msg);
> +    free(ctx);
> +}
> +
> +void libvlc_threads_init (void)
> +{
> +    vlc_mutex_lock (&lock);
> +    if (refs++ == 0)
> +        vlc_threadvar_create (&context, free_context);
> +    vlc_mutex_unlock (&lock);
> +    libvlc_thread_context_t *ctx =
> +        (libvlc_thread_context_t *)calloc (1, sizeof(*ctx));

This seems to be leaked. I cannot see where it gets freed.

> +    vlc_threadvar_set (context, ctx);
> +}
> +
> +void libvlc_threads_deinit (void)
> +{
> +    vlc_mutex_lock (&lock);
> +    assert (refs > 0);
> +    if (--refs == 0)
> +        vlc_threadvar_delete (&context);
> +    vlc_mutex_unlock (&lock);
> +}
> +
> +libvlc_thread_context_t *libvlc_get_thread_context (void)
> +{
> +    return (libvlc_thread_context_t *) vlc_threadvar_get (context);

The caller assumes a non-NULL value. This will crash outside the libvlc_new() 
thread.

> +}
> 
> diff --git a/lib/error.c b/lib/error.c
> index 45397a6..c7af88a 100644
> --- a/lib/error.c
> +++ b/lib/error.c
> @@ -27,38 +27,25 @@
> 
> 
>  static const char oom[] = "Out of memory";
> -/* TODO: use only one thread-specific key for whole libvlc */
> -static vlc_threadvar_t context;
> 
> -static vlc_mutex_t lock = VLC_STATIC_MUTEX;
> -static uintptr_t refs = 0;
> -
> -static void free_msg (void *msg)
> -{
> -    if (msg != oom)
> -        free (msg);
> -}
> -
> -void libvlc_threads_init (void)
> +static char *get_error (void)
>  {
> -    vlc_mutex_lock (&lock);
> -    if (refs++ == 0)
> -        vlc_threadvar_create (&context, free_msg);
> -    vlc_mutex_unlock (&lock);
> +    libvlc_thread_context_t *ctx = libvlc_get_thread_context ();
> +    return ctx->err.msg;
>  }
> 
> -void libvlc_threads_deinit (void)
> +static void free_msg (void *msg)
>  {
> -    vlc_mutex_lock (&lock);
> -    assert (refs > 0);
> -    if (--refs == 0)
> -        vlc_threadvar_delete (&context);
> -    vlc_mutex_unlock (&lock);
> +    if (msg != oom)
> +        free(msg);
>  }
> 
> -static char *get_error (void)
> +static set_error (char *msg)
>  {
> -    return vlc_threadvar_get (context);
> +    libvlc_thread_context_t *ctx = libvlc_get_thread_context ();
> +    if (!ctx->err.free)
> +        ctx->err.free = free_msg;

I don't really see the point in that callback...

> +    ctx->err.msg = msg;
>  }
> 
>  static void free_error (void)
> @@ -86,7 +73,7 @@ const char *libvlc_errmsg (void)
>  void libvlc_clearerr (void)
>  {
>      free_error ();
> -    vlc_threadvar_set (context, NULL);
> +    set_error (NULL);
>  }
> 
>  /**
> @@ -103,7 +90,7 @@ const char *libvlc_vprinterr (const char *fmt, va_list
> ap) msg = (char *)oom;
> 
>      free_error ();
> -    vlc_threadvar_set (context, msg);
> +    set_error (msg);
>      return msg;
>  }
> 
> @@ -122,4 +109,3 @@ const char *libvlc_printerr (const char *fmt, ...)
>      va_end (ap);
>      return msg;
>  }
> -
> diff --git a/lib/libvlc_internal.h b/lib/libvlc_internal.h
> index 64585a7..7690bae 100644
> --- a/lib/libvlc_internal.h
> +++ b/lib/libvlc_internal.h
> @@ -86,10 +86,18 @@ struct libvlc_instance_t
>  
> ***************************************************************************
> /
> 
>  /* Thread context */
> +typedef struct libvlc_thread_context_t
> +{
> +    struct {
> +        char *msg;
> +        void (*free) (void *);
> +    } err;
> +} libvlc_thread_context_t;
>  void libvlc_threads_init (void);
>  void libvlc_threads_deinit (void);
>  void libvlc_log_init (void);
>  void libvlc_log_deinit (void);
> +libvlc_thread_context_t * libvlc_get_thread_context(void);
> 
>  /* Events */
>  libvlc_event_manager_t * libvlc_event_manager_new(

-- 
Rémi Denis-Courmont
http://www.remlab.net/




More information about the vlc-devel mailing list