From e0d06ecedcde2c31904e9ecafd3a42540bfa627b Mon Sep 17 00:00:00 2001 From: Dudemanguy Date: Thu, 7 Aug 2025 18:55:46 -0500 Subject: [PATCH] Revert "m_option: automatically expand paths for M_OPT_FILE options" It would be nice to expand all path arguments for free in the option code, but life isn't so simple. Because of string list options, we have to expand at parsing time. Because of libmpv, mpv_global has to be passed around a million places. Even after doing all that, there's still a drawback where anything defined in builtin.conf (used for setting default profiles and such) will be unable to correctly resolve paths if the user uses --config-dir. Nothing currently defined in builtin.conf would be subject to that and it's not likely that anyone would add anything that would be, but it's a limitation that can't be changed because this has to be parsed before the user's command line and before mpv_global is set. There's also another problem of pretty printing paths which are now always full paths instead of relative ones. It's not so nice if the user's home path is something very long. This could be dealt with by saving an unexpanded path in some field in the opt somewhere and then doing that for pretty printing but with all this extra complication, it's questionable if there's really any net gain at the end of the day. Calling mp_get_user_path on some opt->whatever is a lot simpler and always works. So this commit goes back to the old model along with the following reverts/reapplies. This reverts commit 9b7374ea40e607b61d9c5db9dffc822d52b7df38. --- options/m_config_frontend.c | 2 +- options/m_option.c | 71 +++++++++++-------------------------- options/m_option.h | 17 --------- 3 files changed, 22 insertions(+), 68 deletions(-) diff --git a/options/m_config_frontend.c b/options/m_config_frontend.c index a5e19ba287..fc92a28e72 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_and_expand(config->global, co->opt, co->data, data); + m_option_copy(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 a9be272faf..c53b6536aa 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -43,7 +43,6 @@ #include "misc/node.h" #include "m_option.h" #include "m_config_frontend.h" -#include "path.h" #if HAVE_DOS_PATHS #define OPTION_PATH_SEPARATOR ';' @@ -1271,15 +1270,6 @@ 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) @@ -1313,16 +1303,15 @@ 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, - .expand = expand_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, + .free = free_str, + .set = str_set, + .get = str_get, + .equal = str_equal, }; //////////// String list @@ -1558,8 +1547,7 @@ static int parse_str_list_impl(struct mp_log *log, const m_option_t *opt, return 1; } -static void copy_str_list_impl(struct mpv_global *global, const m_option_t *opt, - void *dst, const void *src) +static void copy_str_list(const m_option_t *opt, void *dst, const void *src) { int n; char **d, **s; @@ -1579,28 +1567,12 @@ static void copy_str_list_impl(struct mpv_global *global, const m_option_t *opt, for (n = 0; s[n] != NULL; n++) /* NOTHING */; d = talloc_array(NULL, char *, n + 1); - for (; n >= 0; n--) { - if (global) { - d[n] = mp_get_user_path(NULL, global, s[n]); - } else { - d[n] = talloc_strdup(NULL, s[n]); - } - } + for (; n >= 0; n--) + 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; @@ -1680,16 +1652,15 @@ 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, - .expand = expand_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, + .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 58fab84212..c6ebdb7978 100644 --- a/options/m_option.h +++ b/options/m_option.h @@ -332,12 +332,6 @@ 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 @@ -581,17 +575,6 @@ 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) {