[vlc-devel] [PATCH] modules: do not alloc/copy in module_list_cap()

[Rémi Denis-Courmont]

If the caller needs to modify the table, it can make its own copy
(which is currently not always the case).

Consequently, the return value can become a size_t.
---
 src/config/core.c     | 16 ++++------------
 src/modules/bank.c    | 19 +++----------------
 src/modules/modules.c | 21 +++++++++++++--------
 src/modules/modules.h | 11 ++++++++++-
 4 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/src/config/core.c b/src/config/core.c
index 8fb7d82fee..ce05d20afd 100644
--- a/src/config/core.c
+++ b/src/config/core.c
@@ -251,14 +251,8 @@ error:
 static ssize_t config_ListModules (const char *cap, char ***restrict values,
                                    char ***restrict texts)
 {
-    module_t **list;
-    ssize_t n = module_list_cap (&list, cap);
-    if (unlikely(n < 0))
-    {
-        *values = *texts = NULL;
-        return n;
-    }
-
+    module_t *const *list;
+    size_t n = module_list_cap(&list, cap);
     char **vals = malloc ((n + 2) * sizeof (*vals));
     char **txts = malloc ((n + 2) * sizeof (*txts));
     if (!vals || !txts)
@@ -269,7 +263,7 @@ static ssize_t config_ListModules (const char *cap, char ***restrict values,
         return -1;
     }
 
-    ssize_t i = 0;
+    size_t i = 0;
 
     vals[i] = strdup ("any");
     txts[i] = strdup (_("Automatic"));
@@ -292,18 +286,16 @@ static ssize_t config_ListModules (const char *cap, char ***restrict values,
 
     *values = vals;
     *texts = txts;
-    module_list_free (list);
     return i + 1;
 
 error:
-    for (ssize_t j = 0; j <= i; ++j)
+    for (size_t j = 0; j <= i; ++j)
     {
         free (vals[j]);
         free (txts[j]);
     }
     free(vals);
     free(txts);
-    module_list_free (list);
     *values = *texts = NULL;
     return -1;
 }
diff --git a/src/modules/bank.c b/src/modules/bank.c
index a880eea6e9..d3f89b60e8 100644
--- a/src/modules/bank.c
+++ b/src/modules/bank.c
@@ -787,15 +787,7 @@ module_t **module_list_get (size_t *n)
     return tab;
 }
 
-/**
- * Builds a sorted list of all VLC modules with a given capability.
- * The list is sorted from the highest module score to the lowest.
- * @param list pointer to the table of modules [OUT]
- * @param name name of capability of modules to look for
- * @return the number of matching found, or -1 on error (*list is then NULL).
- * @note *list must be freed with module_list_free().
- */
-ssize_t module_list_cap (module_t ***restrict list, const char *name)
+size_t module_list_cap(module_t *const **restrict list, const char *name)
 {
     const void **cp = tfind(&name, &modules.caps_tree, vlc_modcap_cmp);
     if (cp == NULL)
@@ -805,12 +797,7 @@ ssize_t module_list_cap (module_t ***restrict list, const char *name)
     }
 
     const vlc_modcap_t *cap = *cp;
-    size_t n = cap->modc;
-    module_t **tab = vlc_alloc (n, sizeof (*tab));
-    *list = tab;
-    if (unlikely(tab == NULL))
-        return -1;
 
-    memcpy(tab, cap->modv, sizeof (*tab) * n);
-    return n;
+    *list = cap->modv;
+    return cap->modc;
 }
diff --git a/src/modules/modules.c b/src/modules/modules.c
index 2df5702f84..700f1ca36b 100644
--- a/src/modules/modules.c
+++ b/src/modules/modules.c
@@ -154,18 +154,23 @@ module_t *(vlc_module_load)(struct vlc_logger *log, const char *capability,
         name = "any";
 
     /* Find matching modules */
-    module_t **mods;
-    ssize_t total = module_list_cap (&mods, capability);
+    module_t *const *tab;
+    size_t total = module_list_cap(&tab, capability);
 
-    vlc_debug(log, "looking for %s module matching \"%s\": %zd candidates",
+    vlc_debug(log, "looking for %s module matching \"%s\": %zu candidates",
               capability, name, total);
-    if (total <= 0)
+    if (total == 0)
     {
-        module_list_free (mods);
         vlc_debug(log, "no %s modules", capability);
         return NULL;
     }
 
+    module_t **mods = malloc(total * sizeof (*mods));
+    if (unlikely(mods == NULL))
+        return NULL;
+
+    memcpy(mods, tab, total * sizeof (*mods));
+
     module_t *module = NULL;
     va_list args;
 
@@ -182,7 +187,7 @@ module_t *(vlc_module_load)(struct vlc_logger *log, const char *capability,
             goto done;
 
         bool force = strict && strcasecmp ("any", shortcut);
-        for (ssize_t i = 0; i < total; i++)
+        for (size_t i = 0; i < total; i++)
         {
             module_t *cand = mods[i];
             if (cand == NULL)
@@ -206,7 +211,7 @@ module_t *(vlc_module_load)(struct vlc_logger *log, const char *capability,
     /* None of the shortcuts matched, fall back to any module */
     if (!strict)
     {
-        for (ssize_t i = 0; i < total; i++)
+        for (size_t i = 0; i < total; i++)
         {
             module_t *cand = mods[i];
             if (cand == NULL || module_get_score (cand) <= 0)
@@ -225,7 +230,7 @@ module_t *(vlc_module_load)(struct vlc_logger *log, const char *capability,
     }
 done:
     va_end (args);
-    module_list_free (mods);
+    free(mods);
 
     if (module != NULL)
         vlc_debug(log, "using %s module \"%s\"", capability,
diff --git a/src/modules/modules.h b/src/modules/modules.h
index 264b40344c..075f6d85f5 100644
--- a/src/modules/modules.h
+++ b/src/modules/modules.h
@@ -113,7 +113,16 @@ void module_EndBank (bool);
 int module_Map(struct vlc_logger *, vlc_plugin_t *);
 void *module_Symbol(struct vlc_logger *, vlc_plugin_t *, const char *name);
 
-ssize_t module_list_cap (module_t ***, const char *);
+/**
+ * Lists of all VLC modules with a given capability.
+ *
+ * The list is sorted by decreasing module score.
+ *
+ * @param list pointer to the table of modules [OUT]
+ * @param name name of capability of modules to look for
+ * @return the number of modules in the list (possibly zero)
+ */
+size_t module_list_cap(module_t *const **, const char *);
 
 int vlc_bindtextdomain (const char *);
 
-- 
2.27.0