From 37c03f2c81a443e910eeb3b2613865dce59c54bf Mon Sep 17 00:00:00 2001 From: wm4 Date: Sat, 4 Aug 2012 11:14:38 +0200 Subject: options: revert passing around talloc contexts This reverts commit 48f0692ab9 "options: make option struct the talloc parent of options". This made things actually more complicated. It introduced a new parameter to the option parse and copy functions, which was used inconsistently. Some code passed a parent, some not. Morever, you have to call m_option_free() anyway, because not all options actually respect the talloc parent. There is also the question whether passing NULL as parent is supposed to work, or if you still have to implement m_config_free(). On the other hand, this simplifies nothing. I assume the intention was being able to free all option values with a single talloc_free() call, but the same goal can be reached by simply freeing the m_config struct. (The m_config talloc destructor will free each option values.) Get rid of the talloc parent context parameter. This essentially reverts commit 48f0692ab9 ("options: make option struct the talloc parent of options"). In video_out.c, make the VO priv struct the talloc parent for the m_config object, so that destroying the VO will free the options. The ability to free the m_config struct and all its managed options was introduced in commit 89a17bcda6c. --- libvo/video_out.c | 2 +- m_config.c | 15 +++------ m_option.c | 91 +++++++++++++++++++++---------------------------------- m_option.h | 11 +++---- m_struct.c | 2 +- 5 files changed, 46 insertions(+), 75 deletions(-) diff --git a/libvo/video_out.c b/libvo/video_out.c index a17fc9aa28..0466ec5fad 100644 --- a/libvo/video_out.c +++ b/libvo/video_out.c @@ -176,11 +176,11 @@ static int vo_preinit(struct vo *vo, char *arg) vo->priv = talloc_zero_size(vo, vo->driver->privsize); if (vo->driver->options) { struct m_config *cfg = m_config_simple(vo->driver->options, vo->priv); + talloc_steal(vo->priv, cfg); char n[50]; int l = snprintf(n, sizeof(n), "vo/%s", vo->driver->info->short_name); assert(l < sizeof(n)); int r = m_config_parse_suboptions(cfg, n, arg); - talloc_free(cfg); if (r < 0) return r; } diff --git a/m_config.c b/m_config.c index 8181ddac37..075adcd633 100644 --- a/m_config.c +++ b/m_config.c @@ -67,8 +67,7 @@ static int parse_profile(struct m_config *config, const struct m_option *opt, } char **list = NULL; - int r = m_option_type_string_list.parse(opt, name, param, false, &list, - NULL); + int r = m_option_type_string_list.parse(opt, name, param, false, &list); if (r < 0) return r; if (!list || !list[0]) @@ -151,7 +150,7 @@ static void optstruct_get(const struct m_config *config, void *dst) { if (opt->type->copy) - opt->type->copy(opt, dst, optstruct_ptr(config, opt), NULL); + opt->type->copy(opt, dst, optstruct_ptr(config, opt)); } static void optstruct_set(const struct m_config *config, @@ -159,11 +158,9 @@ static void optstruct_set(const struct m_config *config, const void *src) { if (opt->type->copy) - opt->type->copy(opt, optstruct_ptr(config, opt), src, config->optstruct); + opt->type->copy(opt, optstruct_ptr(config, opt), src); } - - static void m_config_add_option(struct m_config *config, const struct m_option *arg, const char *prefix, char *disabled_feature); @@ -445,8 +442,7 @@ static int m_config_parse_option(struct m_config *config, void *optstruct, ensure_backup(config, co); void *dst = set ? m_option_get_ptr(co->opt, optstruct) : NULL; - int r = co->opt->type->parse(co->opt, name, param, ambiguous_param, dst, - optstruct); + int r = m_option_parse(co->opt, name, param, ambiguous_param, dst); // Parsing failed ? if (r < 0) return r; @@ -461,8 +457,7 @@ static int parse_subopts(struct m_config *config, void *optstruct, char *name, { char **lst = NULL; // Split the argument into child options - int r = m_option_type_subconfig.parse(NULL, bstr0(""), param, false, &lst, - optstruct); + int r = m_option_type_subconfig.parse(NULL, bstr0(""), param, false, &lst); if (r < 0) return r; // Parse the child options diff --git a/m_option.c b/m_option.c index 8ef4267a9f..283e24b37b 100644 --- a/m_option.c +++ b/m_option.c @@ -77,8 +77,7 @@ const m_option_t *m_option_list_find(const m_option_t *list, const char *name) // Default function that just does a memcpy -static void copy_opt(const m_option_t *opt, void *dst, const void *src, - void *talloc_ctx) +static void copy_opt(const m_option_t *opt, void *dst, const void *src) { if (dst && src) memcpy(dst, src, opt->type->size); @@ -89,8 +88,7 @@ static void copy_opt(const m_option_t *opt, void *dst, const void *src, #define VAL(x) (*(int *)(x)) static int parse_flag(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { if (param.len && !ambiguous_param) { char * const enable[] = { "yes", "on", "ja", "si", "igen", "y", "j", @@ -179,8 +177,7 @@ static int parse_longlong(const m_option_t *opt, struct bstr name, } static int parse_int(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { long long tmp; int r = parse_longlong(opt, name, param, false, &tmp); @@ -190,8 +187,7 @@ static int parse_int(const m_option_t *opt, struct bstr name, } static int parse_int64(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { long long tmp; int r = parse_longlong(opt, name, param, false, &tmp); @@ -225,8 +221,7 @@ const m_option_type_t m_option_type_int64 = { }; static int parse_intpair(const struct m_option *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { if (param.len == 0) return M_OPT_MISSING_PARAM; @@ -269,8 +264,7 @@ const struct m_option_type m_option_type_intpair = { }; static int parse_choice(const struct m_option *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { bool allow_empty = opt->flags & M_OPT_IMPLICIT_DEFAULT; int ret; @@ -326,8 +320,7 @@ const struct m_option_type m_option_type_choice = { #define VAL(x) (*(double *)(x)) static int parse_double(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { if (param.len == 0) return M_OPT_MISSING_PARAM; @@ -401,11 +394,10 @@ const m_option_type_t m_option_type_double = { #define VAL(x) (*(float *)(x)) static int parse_float(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { double tmp; - int r = parse_double(opt, name, param, false, &tmp, NULL); + int r = parse_double(opt, name, param, false, &tmp); if (r == 1 && dst) VAL(dst) = tmp; return r; @@ -431,8 +423,7 @@ const m_option_type_t m_option_type_float = { #define VAL(x) (*(off_t *)(x)) static int parse_position(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { long long tmp; int r = parse_longlong(opt, name, param, false, &tmp); @@ -462,8 +453,7 @@ const m_option_type_t m_option_type_position = { #define VAL(x) (*(char **)(x)) static int parse_str(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { if (param.start == NULL) return M_OPT_MISSING_PARAM; @@ -484,7 +474,7 @@ static int parse_str(const m_option_t *opt, struct bstr name, if (dst) { talloc_free(VAL(dst)); - VAL(dst) = bstrdup0(talloc_ctx, param); + VAL(dst) = bstrdup0(NULL, param); } return 1; @@ -496,12 +486,11 @@ static char *print_str(const m_option_t *opt, const void *val) return (val && VAL(val)) ? talloc_strdup(NULL, VAL(val)) : NULL; } -static void copy_str(const m_option_t *opt, void *dst, const void *src, - void *talloc_ctx) +static void copy_str(const m_option_t *opt, void *dst, const void *src) { if (dst && src) { talloc_free(VAL(dst)); - VAL(dst) = talloc_strdup(talloc_ctx, VAL(src)); + VAL(dst) = talloc_strdup(NULL, VAL(src)); } } @@ -549,7 +538,7 @@ static void free_str_list(void *dst) VAL(dst) = NULL; } -static int str_list_add(char **add, int n, void *dst, int pre, void *talloc_ctx) +static int str_list_add(char **add, int n, void *dst, int pre) { char **lst = VAL(dst); int ln; @@ -653,8 +642,7 @@ static struct bstr get_nextsep(struct bstr *ptr, char sep, bool modify) } static int parse_str_list(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { char **res; int op = OP_NONE; @@ -703,14 +691,14 @@ static int parse_str_list(const m_option_t *opt, struct bstr name, if (!dst) return 1; - res = talloc_array(talloc_ctx, char *, n + 2); + res = talloc_array(NULL, char *, n + 2); str = bstrdup(NULL, param); char *ptr = str.start; n = 0; while (1) { struct bstr el = get_nextsep(&str, separator, 1); - res[n] = bstrdup0(talloc_ctx, el); + res[n] = bstrdup0(NULL, el); n++; if (!str.len) break; @@ -721,9 +709,9 @@ static int parse_str_list(const m_option_t *opt, struct bstr name, switch (op) { case OP_ADD: - return str_list_add(res, n, dst, 0, talloc_ctx); + return str_list_add(res, n, dst, 0); case OP_PRE: - return str_list_add(res, n, dst, 1, talloc_ctx); + return str_list_add(res, n, dst, 1); case OP_DEL: return str_list_del(res, n, dst); } @@ -735,8 +723,7 @@ static int parse_str_list(const m_option_t *opt, struct bstr name, return 1; } -static void copy_str_list(const m_option_t *opt, void *dst, const void *src, - void *talloc_ctx) +static void copy_str_list(const m_option_t *opt, void *dst, const void *src) { int n; char **d, **s; @@ -755,9 +742,9 @@ 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(talloc_ctx, char *, n + 1); + d = talloc_array(NULL, char *, n + 1); for (; n >= 0; n--) - d[n] = talloc_strdup(talloc_ctx, s[n]); + d[n] = talloc_strdup(NULL, s[n]); VAL(dst) = d; } @@ -801,8 +788,7 @@ const m_option_type_t m_option_type_string_list = { /////////////////// Print static int parse_print(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { if (opt->type == CONF_TYPE_PRINT_INDIRECT) mp_msg(MSGT_CFGPARSER, MSGL_INFO, "%s", *(char **) opt->p); @@ -843,8 +829,7 @@ const m_option_type_t m_option_type_print_func = { #define VAL(x) (*(char ***)(x)) static int parse_subconf(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { int nr = 0; char **lst = NULL; @@ -1004,8 +989,7 @@ static struct { }; static int parse_imgfmt(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { uint32_t fmt = 0; int i; @@ -1095,8 +1079,7 @@ static struct { }; static int parse_afmt(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { uint32_t fmt = 0; int i; @@ -1164,8 +1147,7 @@ static int parse_timestring(struct bstr str, double *time, char endchar) static int parse_time(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { double time; @@ -1195,8 +1177,7 @@ const m_option_type_t m_option_type_time = { // Time or size (-endpos) static int parse_time_size(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { m_time_size_t ts; char unit[4]; @@ -1428,8 +1409,7 @@ static int get_obj_params(struct bstr opt_name, struct bstr name, } static int parse_obj_params(const m_option_t *opt, struct bstr name, - struct bstr param, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr param, bool ambiguous_param, void *dst) { char **opts; int r; @@ -1627,7 +1607,7 @@ static void free_obj_settings_list(void *dst) static int parse_obj_settings_list(const m_option_t *opt, struct bstr name, struct bstr param, bool ambiguous_param, - void *dst, void *talloc_ctx) + void *dst) { int len = strlen(opt->name); m_obj_settings_t *res = NULL, *queue = NULL, *head = NULL; @@ -1758,7 +1738,7 @@ static int parse_obj_settings_list(const m_option_t *opt, struct bstr name, } static void copy_obj_settings_list(const m_option_t *opt, void *dst, - const void *src, void *talloc_ctx) + const void *src) { m_obj_settings_t *d, *s; int n; @@ -1781,7 +1761,7 @@ static void copy_obj_settings_list(const m_option_t *opt, void *dst, for (n = 0; s[n].name; n++) { d[n].name = talloc_strdup(NULL, s[n].name); d[n].attribs = NULL; - copy_str_list(NULL, &(d[n].attribs), &(s[n].attribs), NULL); + copy_str_list(NULL, &(d[n].attribs), &(s[n].attribs)); } d[n].name = NULL; d[n].attribs = NULL; @@ -1801,7 +1781,7 @@ const m_option_type_t m_option_type_obj_settings_list = { static int parse_obj_presets(const m_option_t *opt, struct bstr name, struct bstr param, bool ambiguous_param, - void *dst, void *talloc_ctx) + void *dst) { m_obj_presets_t *obj_p = (m_obj_presets_t *)opt->priv; const m_struct_t *in_desc; @@ -1876,8 +1856,7 @@ const m_option_type_t m_option_type_obj_presets = { }; static int parse_custom_url(const m_option_t *opt, struct bstr name, - struct bstr url, bool ambiguous_param, void *dst, - void *talloc_ctx) + struct bstr url, bool ambiguous_param, void *dst) { int r; m_struct_t *desc = opt->priv; diff --git a/m_option.h b/m_option.h index 2584471e7c..d100e1c628 100644 --- a/m_option.h +++ b/m_option.h @@ -192,12 +192,11 @@ struct m_option_type { * may not be an argument meant for this option * \param dst Pointer to the memory where the data should be written. * If NULL the parameter validity should still be checked. - * talloc_ctx: talloc context if value type requires allocations * \return On error a negative value is returned, on success the number * of arguments consumed. For details see \ref OptionParserReturn. */ int (*parse)(const m_option_t *opt, struct bstr name, struct bstr param, - bool ambiguous_param, void *dst, void *talloc_ctx); + bool ambiguous_param, void *dst); // Print back a value in string form. /** \param opt The option to print. @@ -211,10 +210,8 @@ struct m_option_type { /** \param opt The option to copy. * \param dst Pointer to the destination memory. * \param src Pointer to the source memory. - * talloc_ctx: talloc context to use in deep copy */ - void (*copy)(const m_option_t *opt, void *dst, const void *src, - void *talloc_ctx); + void (*copy)(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. @@ -393,7 +390,7 @@ static inline int m_option_parse(const m_option_t *opt, struct bstr name, struct bstr param, bool ambiguous_param, void *dst) { - return opt->type->parse(opt, name, param, ambiguous_param, dst, NULL); + return opt->type->parse(opt, name, param, ambiguous_param, dst); } // Helper to print options, see \ref m_option_type::print. @@ -410,7 +407,7 @@ static inline void m_option_copy(const m_option_t *opt, void *dst, const void *src) { if (opt->type->copy) - opt->type->copy(opt, dst, src, NULL); + opt->type->copy(opt, dst, src); } // Helper around \ref m_option_type::free. diff --git a/m_struct.c b/m_struct.c index 5ee932945a..82cdc0ee27 100644 --- a/m_struct.c +++ b/m_struct.c @@ -71,7 +71,7 @@ int m_struct_set(const m_struct_t *st, void *obj, const char *field, return 0; } - if(f->type->parse(f, bstr0(field), param, false, M_ST_MB_P(obj,f->p), NULL) < 0) { + if(f->type->parse(f, bstr0(field), param, false, M_ST_MB_P(obj,f->p)) < 0) { mp_msg(MSGT_CFGPARSER, MSGL_ERR,"Struct %s, field %s parsing error: %.*s\n", st->name, field, BSTR_P(param)); return 0; -- cgit v1.2.3