Create primary module from plug-in descriptor and factor code

This should ease later implementation of proper error handling.

include/vlc_plugin.h

@@ -36,7 +36,7 @@ VLC_API int vlc_plugin_set(module_t *, module_config_t *, int, ...);
 
 enum vlc_module_properties
 {
-    VLC_SUBMODULE_CREATE,
+    VLC_MODULE_CREATE,
     VLC_CONFIG_CREATE,
 
     /* DO NOT EVER REMOVE, INSERT OR REPLACE ANY ITEM! It would break the ABI!
@@ -110,8 +110,8 @@ enum vlc_module_properties
 /**
  * Current plugin ABI version
  */
-# define MODULE_SYMBOL 1_2_0h
-# define MODULE_SUFFIX "__1_2_0h"
+# define MODULE_SYMBOL 1_2_0i
+# define MODULE_SUFFIX "__1_2_0i"
 
 /*****************************************************************************
  * Add a few defines. You do not want to read this section. Really.
@@ -166,26 +166,28 @@ enum vlc_module_properties
  */
 #define vlc_module_begin() \
 EXTERN_SYMBOL DLL_SYMBOL \
-int CDECL_SYMBOL __VLC_SYMBOL(vlc_entry) (module_t *); \
+module_t *CDECL_SYMBOL __VLC_SYMBOL(vlc_entry) (void); \
 EXTERN_SYMBOL DLL_SYMBOL \
-int CDECL_SYMBOL __VLC_SYMBOL(vlc_entry) (module_t *p_module) \
+module_t *CDECL_SYMBOL __VLC_SYMBOL(vlc_entry) (void) \
 { \
-    module_t *p_submodule; \
+    module_t *module, *p_submodule; \
     module_config_t *p_config = NULL; \
-    if (vlc_module_set (p_module, VLC_MODULE_NAME, (MODULE_STRING))) \
+    if (vlc_plugin_set (NULL, NULL, VLC_MODULE_CREATE, &module)) \
         goto error; \
-    p_submodule = p_module;
+    if (vlc_module_set (module, VLC_MODULE_NAME, (MODULE_STRING))) \
+        goto error; \
+    p_submodule = module;
 
 #define vlc_module_end() \
     (void) p_config; \
-    return VLC_SUCCESS; \
+    return module; \
 error: \
-    return VLC_EGENERIC; \
+    return NULL; \
 } \
 VLC_METADATA_EXPORTS
 
 #define add_submodule( ) \
-    if (vlc_plugin_set (p_module, NULL, VLC_SUBMODULE_CREATE, &p_submodule)) \
+    if (vlc_plugin_set (module, NULL, VLC_MODULE_CREATE, &p_submodule)) \
         goto error;
 
 #define add_shortcut( ... ) \
@@ -227,7 +229,7 @@ VLC_METADATA_EXPORTS
         goto error;
 
 #define set_text_domain( dom ) \
-    if (vlc_module_set (p_module, VLC_MODULE_TEXTDOMAIN, (dom))) \
+    if (vlc_module_set (module, VLC_MODULE_TEXTDOMAIN, (dom))) \
         goto error;
 
 /*****************************************************************************
@@ -244,7 +246,7 @@ VLC_METADATA_EXPORTS
  *****************************************************************************/
 
 #define add_type_inner( type ) \
-    vlc_plugin_set (p_module, NULL, VLC_CONFIG_CREATE, (type), &p_config);
+    vlc_plugin_set (module, NULL, VLC_CONFIG_CREATE, (type), &p_config);
 
 #define add_typedesc_inner( type, text, longtext ) \
     add_type_inner( type ) \

src/modules/cache.c

@@ -199,7 +199,7 @@ size_t CacheLoad( vlc_object_t *p_this, const char *dir, module_cache_t **r )
         uint16_t i_size;
         int i_submodules;
 
-        module = vlc_module_create();
+        module = vlc_module_create (NULL);
 
         /* Load additional infos */
         LOAD_STRING(module->psz_shortname);
@@ -233,7 +233,7 @@ size_t CacheLoad( vlc_object_t *p_this, const char *dir, module_cache_t **r )
 
         while( i_submodules-- )
         {
-            module_t *submodule = vlc_submodule_create (module);
+            module_t *submodule = vlc_module_create (module);
             free (submodule->pp_shortcuts);
             LOAD_STRING(submodule->psz_shortname);
             LOAD_STRING(submodule->psz_longname);

src/modules/entry.c

@@ -39,15 +39,27 @@ static char *strdup_null (const char *str)
     return (str != NULL) ? strdup (str) : NULL;
 }
 
-module_t *vlc_module_create (void)
+module_t *vlc_module_create (module_t *parent)
 {
     module_t *module = malloc (sizeof (*module));
     if (module == NULL)
         return NULL;
 
-    module->next = NULL;
+    /* TODO: replace module/submodules with plugin/modules */
+    if (parent == NULL)
+    {
+        module->next = NULL;
+        module->parent = NULL;
+    }
+    else
+    {
+        module->next = parent->submodule;
+        parent->submodule = module;
+        parent->submodule_count++;
+        module->parent = parent;
+    }
+
     module->submodule = NULL;
-    module->parent = NULL;
     module->submodule_count = 0;
 
     module->psz_shortname = NULL;
@@ -56,8 +68,9 @@ module_t *vlc_module_create (void)
     module->pp_shortcuts = NULL;
     module->i_shortcuts = 0;
     module->psz_capability = NULL;
-    module->i_score = 1;
-    module->b_unloadable = true;
+    module->i_score = (parent != NULL) ? parent->i_score : 1;
+    module->b_loaded = false;
+    module->b_unloadable = parent == NULL;
     module->pf_activate = NULL;
     module->pf_deactivate = NULL;
     module->p_config = NULL;
@@ -67,7 +80,6 @@ module_t *vlc_module_create (void)
     /*module->handle = garbage */
     module->psz_filename = NULL;
     module->domain = NULL;
-    module->b_loaded = false;
     return module;
 }
 
@@ -91,44 +103,6 @@ void vlc_module_destroy (module_t *module)
     free (module);
 }
 
-module_t *vlc_submodule_create (module_t *module)
-{
-    assert (module != NULL);
-
-    module_t *submodule = malloc (sizeof (*submodule));
-    if (unlikely(submodule == NULL))
-        return NULL;
-
-    /* TODO: replace module/submodules with plugin/modules */
-    submodule->next = module->submodule;
-    module->submodule = submodule;
-    module->submodule_count++;
-    submodule->parent = module;
-    submodule->submodule = NULL;
-    submodule->submodule_count = 0;
-
-    submodule->pp_shortcuts = NULL;
-    submodule->i_shortcuts = 0;
-
-    submodule->psz_shortname = NULL;
-    submodule->psz_longname = NULL;
-    submodule->psz_help = NULL;
-    submodule->psz_capability = NULL;
-    submodule->i_score = module->i_score;
-    submodule->b_loaded = false;
-    submodule->b_unloadable = false;
-    submodule->pf_activate = NULL;
-    submodule->pf_deactivate = NULL;
-    submodule->p_config = NULL;
-    submodule->confsize = 0;
-    submodule->i_config_items = 0;
-    submodule->i_bool_items = 0;
-    /*submodule->handle = unused*/
-    /*submodule->psz_filename unused */
-    submodule->domain = NULL;
-    return submodule;
-}
-
 static module_config_t *vlc_config_create (module_t *module, int type)
 {
     unsigned confsize = module->confsize;
@@ -171,10 +145,10 @@ int vlc_plugin_set (module_t *module, module_config_t *item, int propid, ...)
     va_start (ap, propid);
     switch (propid)
     {
-        case VLC_SUBMODULE_CREATE:
+        case VLC_MODULE_CREATE:
         {
             module_t **pp = va_arg (ap, module_t **);
-            module_t *submodule = vlc_submodule_create (module);
+            module_t *submodule = vlc_module_create (module);
 
             if (unlikely(submodule == NULL))
             {
@@ -182,6 +156,9 @@ int vlc_plugin_set (module_t *module, module_config_t *item, int propid, ...)
                 break;
             }
 
+            *pp = submodule;
+            if (module == NULL)
+                break;
             /* Inheritance. Ugly!! */
             submodule->pp_shortcuts = xmalloc (sizeof (char **));
             submodule->pp_shortcuts[0] = strdup_null (module->pp_shortcuts[0]);
@@ -190,7 +167,6 @@ int vlc_plugin_set (module_t *module, module_config_t *item, int propid, ...)
             submodule->psz_shortname = strdup_null (module->psz_shortname);
             submodule->psz_longname = strdup_null (module->psz_longname);
             submodule->psz_capability = strdup_null (module->psz_capability);
-            *pp = submodule;
             break;
         }
 

src/modules/modules.c

@@ -65,7 +65,7 @@ static struct
     unsigned usage;
 } modules = { VLC_STATIC_MUTEX, NULL, 0 };
 
-int vlc_entry__main( module_t * );
+module_t *vlc_entry__main (void);
 
 /*****************************************************************************
  * Local prototypes
@@ -80,7 +80,7 @@ static void AllocatePluginDir( vlc_object_t *, module_bank_t *, const char *,
                                unsigned, cache_mode_t );
 static int  AllocatePluginFile( vlc_object_t *, module_bank_t *, const char *,
                                 const struct stat *, cache_mode_t );
-static module_t * AllocatePlugin( vlc_object_t *, const char *, bool );
+static module_t *module_InitDynamic (vlc_object_t *, const char *, bool);
 #endif
 static module_t *module_InitStatic (vlc_plugin_cb);
 static void DeleteModule (module_t **, module_t *);
@@ -514,7 +514,7 @@ found_shortcut:
             assert (p_real->psz_filename != NULL);
 
             module_t *p_new_module =
-                AllocatePlugin( p_this, p_real->psz_filename, false );
+                module_InitDynamic (p_this, p_real->psz_filename, false);
             if( p_new_module == NULL )
             {   /* Corrupted module */
                 msg_Err( p_this, "possibly corrupt module cache" );
@@ -918,7 +918,7 @@ static int AllocatePluginFile( vlc_object_t * p_this, module_bank_t *p_bank,
         p_module = CacheFind (p_bank->loaded_cache, p_bank->i_loaded_cache,
                               path, st);
     if( p_module == NULL )
-        p_module = AllocatePlugin( p_this, path, true );
+        p_module = module_InitDynamic (p_this, path, true);
     if( p_module == NULL )
         return -1;
 
@@ -941,7 +941,7 @@ static int AllocatePluginFile( vlc_object_t * p_this, module_bank_t *p_bank,
              /* !unloadable not allowed for plugins with callbacks */
              assert( !p_module->b_loaded );
              DeleteModule (&modules.head, p_module);
-             p_module = AllocatePlugin( p_this, path, false );
+             p_module = module_InitDynamic (p_this, path, false);
              break;
          }
 
@@ -957,62 +957,54 @@ static int AllocatePluginFile( vlc_object_t * p_this, module_bank_t *p_bank,
     return  0;
 }
 
-/*****************************************************************************
- * AllocatePlugin: load a module into memory and initialize it.
- *****************************************************************************
- * This function loads a dynamically loadable module and allocates a structure
- * for its information data. The module can then be handled by module_need
- * and module_unneed. It can be removed by DeleteModule.
- *****************************************************************************/
-static module_t *AllocatePlugin( vlc_object_t * p_this, const char *psz_file,
-                                 bool fast )
+/**
+ * Loads a dynamically-linked plug-in into memory and initialize it.
+ *
+ * The module can then be handled by module_need() and module_unneed().
+ * It can be removed by DeleteModule.
+ *
+ * \param path file path of the shared object
+ * \param fast whether to optimize loading for speed or safety
+ *             (fast is used when the plug-in is registered but not used)
+ */
+static module_t *module_InitDynamic (vlc_object_t *obj,
+                                     const char *path, bool fast)
 {
-    module_t * p_module = NULL;
     module_handle_t handle;
 
-    if( module_Load( p_this, psz_file, &handle, fast ) )
-        return NULL;
-
-    /* Now that we have successfully loaded the module, we can
-     * allocate a structure for it */
-    p_module = vlc_module_create();
-    if( p_module == NULL )
-    {
-        module_Unload( handle );
+    if (module_Load (obj, path, &handle, fast))
         return NULL;
-    }
-
-    p_module->psz_filename = strdup (psz_file);
-    if (unlikely(p_module->psz_filename == NULL))
-        goto error;
-    p_module->handle = handle;
-    p_module->b_loaded = true;
-
-    /* Initialize the module: fill p_module, default config */
-    static const char entry[] = "vlc_entry" MODULE_SUFFIX;
 
     /* Try to resolve the symbol */
-    int (*pf_symbol)(module_t * p_module)
-        = (int (*)(module_t *)) module_Lookup( p_module->handle,entry );
-    if( pf_symbol == NULL )
+    static const char entry_name[] = "vlc_entry" MODULE_SUFFIX;
+    vlc_plugin_cb entry =
+        (vlc_plugin_cb) module_Lookup (handle, entry_name);
+    if (entry == NULL)
     {
-        msg_Warn( p_this, "cannot find symbol \"%s\" in plugin `%s'",
-                  entry, psz_file );
+        msg_Warn (obj, "cannot find plug-in entry point in %s", path);
         goto error;
     }
-    else
+
     /* We can now try to call the symbol */
-    if( pf_symbol( p_module ) != 0 )
+    module_t *module = entry ();
+    if (unlikely(module == NULL))
     {
         /* With a well-written module we shouldn't have to print an
          * additional error message here, but just make sure. */
-        msg_Err( p_this, "cannot initialize plugin `%s'", psz_file );
+        msg_Err (obj, "cannot initialize plug-in %s", path);
         goto error;
     }
-    return p_module;
+
+    module->psz_filename = strdup (path);
+    if (unlikely(module->psz_filename == NULL))
+    {
+        vlc_module_destroy (module);
+        goto error;
+    }
+    module->handle = handle;
+    module->b_loaded = true;
+    return module;
 error:
-    free( p_module->psz_filename );
-    vlc_module_destroy (p_module);
     module_Unload( handle );
     return NULL;
 }
@@ -1023,13 +1015,10 @@ error:
  */
 static module_t *module_InitStatic (vlc_plugin_cb entry)
 {
-    module_t *module = vlc_module_create ();
-    if (unlikely(module == NULL))
-        return NULL;
-
     /* Initializes the module */
-    if (entry (module))
-        assert (0);
+    module_t *module = entry ();
+    if (unlikely (module == NULL))
+        abort (); /* FIXME: OOM or bug */
 
     module->b_loaded = true;
     module->b_unloadable = false;

src/modules/modules.h

@@ -46,7 +46,7 @@ struct module_cache_t
 /** The module handle type */
 typedef void *module_handle_t;
 
-typedef int (*vlc_plugin_cb) (module_t *);
+typedef module_t *(*vlc_plugin_cb) (void);
 
 /**
  * Internal module descriptor
@@ -96,8 +96,7 @@ struct module_t
     char *              domain;                            /* gettext domain */
 };
 
-module_t *vlc_module_create (void);
-module_t *vlc_submodule_create (module_t *module);
+module_t *vlc_module_create (module_t *);
 void vlc_module_destroy (module_t *);
 
 void module_InitBank (void);