From 455af6aa68a96fa38f17174c35e95e13098886e0 Mon Sep 17 00:00:00 2001 From: wm4 Date: Mon, 21 May 2018 14:34:17 +0200 Subject: [PATCH] m_config: optimize initialization of each option Options with dynamic memory allocations (such as strings) require some care. They need to be fully copied on initialization, and if a static default value was declared, we must not free that value either. Instead of going through the entire thing even for simple types like integers, really run it only for options with dynamic allocations. To distinguish types which use dynamic allocations, we can use the fact that they require a free callback (otherwise they would leak). As a result initialization of simple types becomes chaper, and the init function does nothing at all if src==dst for a simple type. (It's funny how mplayer had M_OPT_TYPE_DYNAMIC since 2002, until we replaced it by the same heuristic as used here in commit 3bb134969eb6. It's also funny how the new check was used only for some asserts, and finally removed in commit 7539928c1c. I guess at this time I felt like having uniform code was more important than pointless micro-optimizations.) The src==NULL case is removed because it can't happen. --- options/m_config.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/options/m_config.c b/options/m_config.c index e17803d023..f7d0b282db 100644 --- a/options/m_config.c +++ b/options/m_config.c @@ -178,15 +178,19 @@ static void substruct_write_ptr(void *ptr, void *val) } // Initialize a field with a given value. In case this is dynamic data, it has -// to be allocated and copied. src can alias dst, also can be NULL. +// to be allocated and copied. src can alias dst. static void init_opt_inplace(const struct m_option *opt, void *dst, const void *src) { - union m_option_value temp = {0}; - if (src) + // The option will use dynamic memory allocation iff it has a free callback. + if (opt->type->free) { + union m_option_value temp; memcpy(&temp, src, opt->type->size); - memset(dst, 0, opt->type->size); - m_option_copy(opt, dst, &temp); + memset(dst, 0, opt->type->size); + m_option_copy(opt, dst, &temp); + } else if (src != dst) { + memcpy(dst, src, opt->type->size); + } } static void alloc_group(struct m_config_data *data, int group_index,