From 9b7374ea40e607b61d9c5db9dffc822d52b7df38 Mon Sep 17 00:00:00 2001 From: Dudemanguy Date: Sat, 12 Jul 2025 17:09:13 -0500 Subject: [PATCH] m_option: automatically expand paths for M_OPT_FILE options Unfortunately, I've already done most of the legwork on expanding paths manually in the code everytime we encounter one of these options, but it is completely possible to do it during the option parsing/copying. All option value setting should go through m_config_set_option_raw at some point, so this is a good target place to put this logic. Additionally, the M_OPT_FILE flag already exists for any option that is meant to be have path values as arguments, so we just needed to add an m_option method to do this. Fundamentally, this is exactly the same as m_option_copy except that we potentially expand the value string. A new expand function pointer is now added and we implement it for the two option types where this is relevant: OPT_STRING and OPT_PATHLIST. Furthermore, a m_option_copy_and_expand wrapper is added to handle which function pointer should be used. With this change, this means that all path values located in any opt->value pointer should automatically be expanded and users of the value no longer have to manually do this themselves. Of course, that means that there's now a ton of redundancy in the code that can be eliminated. That will be cleaned up in the next commits. --- options/m_config_frontend.c | 2 +- options/m_option.c | 71 ++++++++++++++++++++++++++----------- options/m_option.h | 17 +++++++++ 3 files changed, 68 insertions(+), 22 deletions(-) diff --git a/options/m_config_frontend.c b/options/m_config_frontend.c index fc92a28e72..a5e19ba287 100644 --- a/options/m_config_frontend.c +++ b/options/m_config_frontend.c @@ -635,7 +635,7 @@ int m_config_set_option_raw(struct m_config *config, m_config_mark_co_flags(co, flags); - m_option_copy(co->opt, co->data, data); + m_option_copy_and_expand(config->global, co->opt, co->data, data); if (m_config_cache_write_opt(config->cache, co->data)) force_self_notify_change_opt(config, co, false); diff --git a/options/m_option.c b/options/m_option.c index c53b6536aa..a9be272faf 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -43,6 +43,7 @@ #include "misc/node.h" #include "m_option.h" #include "m_config_frontend.h" +#include "path.h" #if HAVE_DOS_PATHS #define OPTION_PATH_SEPARATOR ';' @@ -1270,6 +1271,15 @@ static void copy_str(const m_option_t *opt, void *dst, const void *src) talloc_replace(NULL, VAL(dst), VAL(src)); } +static void expand_str(struct mpv_global *global, const m_option_t *opt, + void *dst, const void *src) +{ + if (dst && src) { + talloc_free(VAL(dst)); + VAL(dst) = mp_get_user_path(NULL, global, VAL(src)); + } +} + static int str_set(const m_option_t *opt, void *dst, struct mpv_node *src) { if (src->format != MPV_FORMAT_STRING) @@ -1303,15 +1313,16 @@ static void free_str(void *src) } const m_option_type_t m_option_type_string = { - .name = "String", - .size = sizeof(char *), - .parse = parse_str, - .print = print_str, - .copy = copy_str, - .free = free_str, - .set = str_set, - .get = str_get, - .equal = str_equal, + .name = "String", + .size = sizeof(char *), + .parse = parse_str, + .print = print_str, + .copy = copy_str, + .expand = expand_str, + .free = free_str, + .set = str_set, + .get = str_get, + .equal = str_equal, }; //////////// String list @@ -1547,7 +1558,8 @@ static int parse_str_list_impl(struct mp_log *log, const m_option_t *opt, return 1; } -static void copy_str_list(const m_option_t *opt, void *dst, const void *src) +static void copy_str_list_impl(struct mpv_global *global, const m_option_t *opt, + void *dst, const void *src) { int n; char **d, **s; @@ -1567,12 +1579,28 @@ static void copy_str_list(const m_option_t *opt, void *dst, const void *src) for (n = 0; s[n] != NULL; n++) /* NOTHING */; d = talloc_array(NULL, char *, n + 1); - for (; n >= 0; n--) - d[n] = talloc_strdup(NULL, s[n]); + for (; n >= 0; n--) { + if (global) { + d[n] = mp_get_user_path(NULL, global, s[n]); + } else { + d[n] = talloc_strdup(NULL, s[n]); + } + } VAL(dst) = d; } +static void copy_str_list(const m_option_t *opt, void *dst, const void *src) +{ + copy_str_list_impl(NULL, opt, dst, src); +} + +static void expand_str_list(struct mpv_global *global, const m_option_t *opt, + void *dst, const void *src) +{ + copy_str_list_impl(global, opt, dst, src); +} + static char *print_str_list(const m_option_t *opt, const void *src) { char **lst = NULL; @@ -1652,15 +1680,16 @@ static bool str_list_equal(const m_option_t *opt, void *a, void *b) } const m_option_type_t m_option_type_string_list = { - .name = "String list", - .size = sizeof(char **), - .parse = parse_str_list, - .print = print_str_list, - .copy = copy_str_list, - .free = free_str_list, - .get = str_list_get, - .set = str_list_set, - .equal = str_list_equal, + .name = "String list", + .size = sizeof(char **), + .parse = parse_str_list, + .print = print_str_list, + .copy = copy_str_list, + .expand = expand_str_list, + .free = free_str_list, + .get = str_list_get, + .set = str_list_set, + .equal = str_list_equal, .actions = (const struct m_option_action[]){ {"add"}, {"append"}, diff --git a/options/m_option.h b/options/m_option.h index c6ebdb7978..58fab84212 100644 --- a/options/m_option.h +++ b/options/m_option.h @@ -332,6 +332,12 @@ struct m_option_type { */ void (*copy)(const m_option_t *opt, void *dst, const void *src); + // Exactly the same as copy but will potentially perform a path expansion if + // the underlying option is a string with paths (M_OPT_FILE). This is for + // options that take paths as argument values. + void (*expand)(struct mpv_global *global, const m_option_t *opt, + void *dst, const void *src); + // Free the data allocated for a save slot. /** This is only needed for dynamic types like strings. * \param dst Pointer to the data, usually a pointer that should be freed and @@ -575,6 +581,17 @@ static inline void m_option_copy(const m_option_t *opt, void *dst, opt->type->copy(opt, dst, src); } +// Expand and copy the string if M_OPT_FILE. Do a raw copy otherwise. +static inline void m_option_copy_and_expand(struct mpv_global *global, const m_option_t *opt, + void *dst, const void *src) +{ + if (opt->flags & M_OPT_FILE && opt->type->expand) { + opt->type->expand(global, opt, dst, src); + } else { + m_option_copy(opt, dst, src); + } +} + // Helper around \ref m_option_type::free. static inline void m_option_free(const m_option_t *opt, void *dst) {