From 8dba805732a431558c1218104dfeb667d9b5df63 Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9mi=20Denis-Courmont?= Date: Fri, 16 May 2008 17:59:03 +0300 Subject: [PATCH] Hide i_children and pp_children away They can only be read safely under the Big Structure Lock in src/misc/objects.c, so it makes no sense for them to be public. By the way, making i_children volatile wouldn't magically solve thread-safety issues. The only correct use of volatile is in dealing with asynchronous changes _within_ the same thread, such as signal handling. P.S.: I wish modules were not objects, and I wonder why they are (they don't don't use threads, nor plugins, nor variables) --- include/vlc_common.h | 2 - src/libvlc-common.c | 2 +- src/libvlc.h | 3 ++ src/misc/objects.c | 106 ++++++++++++++++++++---------------------- src/modules/cache.c | 13 +++--- src/modules/modules.c | 16 +++---- 6 files changed, 69 insertions(+), 73 deletions(-) diff --git a/include/vlc_common.h b/include/vlc_common.h index 03fd5037c3..ef59010854 100644 --- a/include/vlc_common.h +++ b/include/vlc_common.h @@ -481,8 +481,6 @@ typedef struct vlc_object_internals_t vlc_object_internals_t; libvlc_int_t *p_libvlc; /**< (root of all evil) - 1 */ \ \ vlc_object_t * p_parent; /**< our parent */ \ - vlc_object_t ** pp_children; /**< our children */ \ - volatile int i_children; \ \ /* Private data */ \ void * p_private; \ diff --git a/src/libvlc-common.c b/src/libvlc-common.c index f2e56131df..26d58a8b9d 100644 --- a/src/libvlc-common.c +++ b/src/libvlc-common.c @@ -426,7 +426,7 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int i_argc, } msg_Dbg( p_libvlc, "module bank initialized, found %i modules", - p_libvlc_global->p_module_bank->i_children ); + vlc_internals( p_libvlc_global->p_module_bank )->i_children ); /* Check for help on modules */ if( (p_tmp = config_GetPsz( p_libvlc, "module" )) ) diff --git a/src/libvlc.h b/src/libvlc.h index bd7b96b4b9..a3ef427671 100644 --- a/src/libvlc.h +++ b/src/libvlc.h @@ -181,6 +181,9 @@ struct vlc_object_internals_t vlc_spinlock_t ref_spin; unsigned i_refcount; vlc_destructor_t pf_destructor; + + vlc_object_t **pp_children; + int i_children; }; #define ZOOM_SECTION N_("Zoom") diff --git a/src/misc/objects.c b/src/misc/objects.c index 851f1bdcd1..b903e6509c 100644 --- a/src/misc/objects.c +++ b/src/misc/objects.c @@ -159,8 +159,8 @@ void *vlc_custom_create( vlc_object_t *p_this, size_t i_size, p_priv->pf_destructor = NULL; p_priv->b_thread = false; p_new->p_parent = NULL; - p_new->pp_children = NULL; - p_new->i_children = 0; + p_priv->pp_children = NULL; + p_priv->i_children = 0; p_new->p_private = NULL; @@ -322,23 +322,23 @@ static void vlc_object_destroy( vlc_object_t *p_this ) p_priv->pf_destructor( p_this ); /* Sanity checks */ - if( p_this->i_children ) + if( p_priv->i_children ) { int i; fprintf( stderr, "ERROR: cannot delete object (%i, %s) with %d children\n", p_this->i_object_id, p_this->psz_object_name, - p_this->i_children ); + p_priv->i_children ); - for( i = 0; i < p_this->i_children; i++ ) + for( i = 0; i < p_priv->i_children; i++ ) { fprintf( stderr, "ERROR: Remaining children object " "(id:%i, type:%s, name:%s)\n", - p_this->pp_children[i]->i_object_id, - p_this->pp_children[i]->psz_object_type, - p_this->pp_children[i]->psz_object_name ); + p_priv->pp_children[i]->i_object_id, + p_priv->pp_children[i]->psz_object_type, + p_priv->pp_children[i]->psz_object_name ); } fflush(stderr); @@ -652,8 +652,8 @@ void __vlc_object_kill( vlc_object_t *p_this ) } if( p_this->i_object_type == VLC_OBJECT_LIBVLC ) - for( int i = 0; i < p_this->i_children ; i++ ) - vlc_object_kill( p_this->pp_children[i] ); + for( int i = 0; i < internals->i_children ; i++ ) + vlc_object_kill( internals->pp_children[i] ); vlc_object_signal_unlocked( p_this ); vlc_mutex_unlock( &p_this->object_lock ); @@ -880,8 +880,8 @@ void __vlc_object_release( vlc_object_t *p_this ) if (p_this->p_parent) vlc_object_detach_unlocked (p_this); /* Detach from children to protect against FIND_PARENT */ - for (int i = 0; i < p_this->i_children; i++) - p_this->pp_children[i]->p_parent = NULL; + for (int i = 0; i < internals->i_children; i++) + internals->pp_children[i]->p_parent = NULL; } vlc_mutex_unlock( &structure_lock ); @@ -908,8 +908,9 @@ void __vlc_object_attach( vlc_object_t *p_this, vlc_object_t *p_parent ) p_this->p_parent = p_parent; /* Attach the child to its parent */ - INSERT_ELEM( p_parent->pp_children, p_parent->i_children, - p_parent->i_children, p_this ); + vlc_object_internals_t *priv = vlc_internals( p_parent ); + INSERT_ELEM( priv->pp_children, priv->i_children, priv->i_children, + p_this ); vlc_mutex_unlock( &structure_lock ); } @@ -919,35 +920,34 @@ static void vlc_object_detach_unlocked (vlc_object_t *p_this) { assert (p_this->p_parent); - vlc_object_t *p_parent = p_this->p_parent; + vlc_object_internals_t *priv = vlc_internals( p_this->p_parent ); + int i_index, i; /* Remove p_this's parent */ p_this->p_parent = NULL; /* Remove all of p_parent's children which are p_this */ - for( i_index = p_parent->i_children ; i_index-- ; ) + for( i_index = priv->i_children ; i_index-- ; ) { - if( p_parent->pp_children[i_index] == p_this ) + if( priv->pp_children[i_index] == p_this ) { - p_parent->i_children--; - for( i = i_index ; i < p_parent->i_children ; i++ ) - { - p_parent->pp_children[i] = p_parent->pp_children[i+1]; - } + priv->i_children--; + for( i = i_index ; i < priv->i_children ; i++ ) + priv->pp_children[i] = priv->pp_children[i+1]; } } - if( p_parent->i_children ) + if( priv->i_children ) { - p_parent->pp_children = (vlc_object_t **)realloc( p_parent->pp_children, - p_parent->i_children * sizeof(vlc_object_t *) ); + priv->pp_children = (vlc_object_t **)realloc( priv->pp_children, + priv->i_children * sizeof(vlc_object_t *) ); } else { /* Special case - don't realloc() to zero to avoid leaking */ - free( p_parent->pp_children ); - p_parent->pp_children = NULL; + free( priv->pp_children ); + priv->pp_children = NULL; } } @@ -1029,13 +1029,14 @@ vlc_list_t * __vlc_list_find( vlc_object_t *p_this, int i_type, int i_mode ) vlc_list_t *__vlc_list_children( vlc_object_t *obj ) { vlc_list_t *l; + vlc_object_internals_t *priv = vlc_internals( obj ); vlc_mutex_lock( &structure_lock ); - l = NewList( obj->i_children ); + l = NewList( priv->i_children ); for (int i = 0; i < l->i_count; i++) { - vlc_object_yield( obj->pp_children[i] ); - l->p_values[i].p_object = obj->pp_children[i]; + vlc_object_yield( priv->pp_children[i] ); + l->p_values[i].p_object = priv->pp_children[i]; } vlc_mutex_unlock( &structure_lock ); return l; @@ -1304,15 +1305,15 @@ static vlc_object_t * FindObject( vlc_object_t *p_this, int i_type, int i_mode ) break; case FIND_CHILD: - for( i = p_this->i_children; i--; ) + for( i = vlc_internals( p_this )->i_children; i--; ) { - p_tmp = p_this->pp_children[i]; + p_tmp = vlc_internals( p_this )->pp_children[i]; if( p_tmp->i_object_type == i_type ) { vlc_object_yield( p_tmp ); return p_tmp; } - else if( p_tmp->i_children ) + else if( vlc_internals( p_tmp )->i_children ) { p_tmp = FindObject( p_tmp, i_type, i_mode ); if( p_tmp ) @@ -1358,16 +1359,16 @@ static vlc_object_t * FindObjectName( vlc_object_t *p_this, break; case FIND_CHILD: - for( i = p_this->i_children; i--; ) + for( i = vlc_internals( p_this )->i_children; i--; ) { - p_tmp = p_this->pp_children[i]; + p_tmp = vlc_internals( p_this )->pp_children[i]; if( p_tmp->psz_object_name && !strcmp( p_tmp->psz_object_name, psz_name ) ) { vlc_object_yield( p_tmp ); return p_tmp; } - else if( p_tmp->i_children ) + else if( vlc_internals( p_tmp )->i_children ) { p_tmp = FindObjectName( p_tmp, psz_name, i_mode ); if( p_tmp ) @@ -1401,7 +1402,7 @@ static void PrintObject( vlc_object_t *p_this, const char *psz_prefix ) } psz_children[0] = '\0'; - switch( p_this->i_children ) + switch( vlc_internals( p_this )->i_children ) { case 0: break; @@ -1409,7 +1410,8 @@ static void PrintObject( vlc_object_t *p_this, const char *psz_prefix ) strcpy( psz_children, ", 1 child" ); break; default: - snprintf( psz_children, 19, ", %i children", p_this->i_children ); + snprintf( psz_children, 19, ", %i children", + vlc_internals( p_this )->i_children ); break; } @@ -1449,7 +1451,7 @@ static void DumpStructure( vlc_object_t *p_this, int i_level, char *psz_foo ) return; } - for( i = 0 ; i < p_this->i_children ; i++ ) + for( i = 0 ; i < vlc_internals( p_this )->i_children ; i++ ) { if( i_level ) { @@ -1461,7 +1463,7 @@ static void DumpStructure( vlc_object_t *p_this, int i_level, char *psz_foo ) } } - if( i == p_this->i_children - 1 ) + if( i == vlc_internals( p_this )->i_children - 1 ) { psz_foo[i_level] = '`'; } @@ -1473,7 +1475,8 @@ static void DumpStructure( vlc_object_t *p_this, int i_level, char *psz_foo ) psz_foo[i_level+1] = '-'; psz_foo[i_level+2] = '\0'; - DumpStructure( p_this->pp_children[i], i_level + 2, psz_foo ); + DumpStructure( vlc_internals( p_this )->pp_children[i], i_level + 2, + psz_foo ); } } @@ -1546,19 +1549,15 @@ static int CountChildren( vlc_object_t *p_this, int i_type ) vlc_object_t *p_tmp; int i, i_count = 0; - for( i = 0; i < p_this->i_children; i++ ) + for( i = 0; i < vlc_internals( p_this )->i_children; i++ ) { - p_tmp = p_this->pp_children[i]; + p_tmp = vlc_internals( p_this )->pp_children[i]; if( p_tmp->i_object_type == i_type ) { i_count++; } - - if( p_tmp->i_children ) - { - i_count += CountChildren( p_tmp, i_type ); - } + i_count += CountChildren( p_tmp, i_type ); } return i_count; @@ -1569,18 +1568,13 @@ static void ListChildren( vlc_list_t *p_list, vlc_object_t *p_this, int i_type ) vlc_object_t *p_tmp; int i; - for( i = 0; i < p_this->i_children; i++ ) + for( i = 0; i < vlc_internals( p_this )->i_children; i++ ) { - p_tmp = p_this->pp_children[i]; + p_tmp = vlc_internals( p_this )->pp_children[i]; if( p_tmp->i_object_type == i_type ) - { ListReplace( p_list, p_tmp, p_list->i_count++ ); - } - if( p_tmp->i_children ) - { - ListChildren( p_list, p_tmp, i_type ); - } + ListChildren( p_list, p_tmp, i_type ); } } diff --git a/src/modules/cache.c b/src/modules/cache.c index e15e890ff1..cc887e9b34 100644 --- a/src/modules/cache.c +++ b/src/modules/cache.c @@ -34,6 +34,7 @@ #include /* free(), strtol() */ #include /* sprintf() */ #include /* strdup() */ +#include #ifdef HAVE_SYS_TYPES_H # include @@ -562,14 +563,14 @@ void CacheSave( vlc_object_t *p_this ) SAVE_STRING( pp_cache[i]->p_module->psz_filename ); - i_submodule = pp_cache[i]->p_module->i_children; + i_submodule = vlc_internals( pp_cache[i]->p_module )->i_children; SAVE_IMMEDIATE( i_submodule ); for( i_submodule = 0; - i_submodule < (unsigned)pp_cache[i]->p_module->i_children; + i_submodule < (unsigned)vlc_internals( pp_cache[i]->p_module)->i_children; i_submodule++ ) { module_t *p_module = - (module_t *)pp_cache[i]->p_module->pp_children[i_submodule]; + (module_t *)vlc_internals( pp_cache[i]->p_module )->pp_children[i_submodule]; SAVE_STRING( p_module->psz_object_name ); SAVE_STRING( p_module->psz_shortname ); @@ -686,10 +687,10 @@ void CacheMerge( vlc_object_t *p_this, module_t *p_cache, module_t *p_module ) p_cache->pf_deactivate = p_module->pf_deactivate; p_cache->handle = p_module->handle; - for( i_submodule = 0; i_submodule < p_module->i_children; i_submodule++ ) + for( i_submodule = 0; i_submodule < vlc_internals( p_module )->i_children; i_submodule++ ) { - module_t *p_child = (module_t*)p_module->pp_children[i_submodule]; - module_t *p_cchild = (module_t*)p_cache->pp_children[i_submodule]; + module_t *p_child = (module_t*)vlc_internals( p_module )->pp_children[i_submodule]; + module_t *p_cchild = (module_t*)vlc_internals( p_cache )->pp_children[i_submodule]; p_cchild->pf_activate = p_child->pf_activate; p_cchild->pf_deactivate = p_child->pf_deactivate; } diff --git a/src/modules/modules.c b/src/modules/modules.c index 62efe31b44..85ae61900a 100644 --- a/src/modules/modules.c +++ b/src/modules/modules.c @@ -214,9 +214,9 @@ void __module_EndBank( vlc_object_t *p_this ) vlc_object_detach( p_libvlc_global->p_module_bank ); - while( p_libvlc_global->p_module_bank->i_children ) + while( vlc_internals( p_libvlc_global->p_module_bank )->i_children ) { - p_next = (module_t *)p_libvlc_global->p_module_bank->pp_children[0]; + p_next = (module_t *)vlc_internals( p_libvlc_global->p_module_bank )->pp_children[0]; DeleteModule( p_next, true ); } @@ -1290,9 +1290,9 @@ static void DupModule( module_t *p_module ) p_module->psz_help = p_module->psz_help ? strdup( p_module->psz_help ) : NULL; - for( i_submodule = 0; i_submodule < p_module->i_children; i_submodule++ ) + for( i_submodule = 0; i_submodule < vlc_internals( p_module )->i_children; i_submodule++ ) { - DupModule( (module_t*)p_module->pp_children[ i_submodule ] ); + DupModule( (module_t*)vlc_internals( p_module )->pp_children[ i_submodule ] ); } } @@ -1306,9 +1306,9 @@ static void UndupModule( module_t *p_module ) char **pp_shortcut; int i_submodule; - for( i_submodule = 0; i_submodule < p_module->i_children; i_submodule++ ) + for( i_submodule = 0; i_submodule < vlc_internals( p_module )->i_children; i_submodule++ ) { - UndupModule( (module_t*)p_module->pp_children[ i_submodule ] ); + UndupModule( (module_t*)vlc_internals( p_module )->pp_children[ i_submodule ] ); } for( pp_shortcut = p_module->pp_shortcuts ; *pp_shortcut ; pp_shortcut++ ) @@ -1393,9 +1393,9 @@ static void DeleteModule( module_t * p_module, bool b_detach ) #endif /* Free and detach the object's children */ - while( p_module->i_children ) + while( vlc_internals( p_module )->i_children ) { - vlc_object_t *p_this = p_module->pp_children[0]; + vlc_object_t *p_this = vlc_internals( p_module )->pp_children[0]; vlc_object_detach( p_this ); vlc_object_release( p_this ); } -- 2.39.2