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

@@ -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;                                              \

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" )) )

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")

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-- ; )
-    {
-        if( p_parent->pp_children[i_index] == p_this )
+    for( i_index = priv->i_children ; i_index-- ; )
     {
-            p_parent->i_children--;
-            for( i = i_index ; i < p_parent->i_children ; i++ )
+        if( priv->pp_children[i_index] == p_this )
         {
-                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,20 +1549,16 @@ 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 );
     }
-    }
 
     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 );
     }
-    }
 }

src/modules/cache.c

@@ -34,6 +34,7 @@
 #include <stdlib.h>                                      /* free(), strtol() */
 #include <stdio.h>                                              /* sprintf() */
 #include <string.h>                                              /* strdup() */
+#include <vlc_plugin.h>
 
 #ifdef HAVE_SYS_TYPES_H
 #   include <sys/types.h>
@@ -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;
     }

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 );
     }