[vlc-devel] [PATCH] modules: do not alloc/copy in module_list_cap()
Thomas Guillem
thomas at gllm.fr
Wed Jul 1 09:14:35 CEST 2020
LGTM.
On Tue, Jun 30, 2020, at 17:44, Rémi Denis-Courmont wrote:
> 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
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list