[MPlayer-dev-eng] CONF_TYPE_STRING issue

[Reimar Döffinger]

On Thu, Apr 28, 2011 at 10:53:07AM +0200, Ingo Brückl wrote:
> There are CONF_TYPE_STRING options in the GUI's config whose associated
> char *variables get freed and strdup'ed by the GUI to change them.
> 
> To investigate whether this is ok (which I assumed might not be), I added
> some printfs to the config/option code (which unfortunately is very hard
> work) to see how/where allocated memory for CONF_TYPE_STRING options is
> beeing freed. I was very surprised to find no such freeing, so either it's
> a memory leak bug or I am doing something wrong.
> 
> For example, I set name="foobar" in my user config and got:
> 
>   # (copy_str, strdup) "foobar" at 0x91c9188
>   # (before m_config_free) vo_winname = "foobar" at 0x91c9188
>   # (m_option_free) name="foobar" at 0x91c9188
>   # (free_str) "foobar" at 0x91c68d0
>   # (m_option_free) name="foobar" at 0x91c9188
> ### don't know why it's called twice, but 0x91c68d0 already freed, so no action
>   # (after m_config_free) vo_winname = "foobar" at 0x91c9188
>   # (manually freeing vo_winname now...)
>   # (after freeing vo_winname) vo_winname = "^P^Har" at 0x91c9188
> 
> So we see that the string "foobar" is created at 0x91c9188 and is still at
> that address before m_config_free() starts. m_option_free() is called, but
> then a "foobar" (slot?) at a different address (0x91c68d0) is freed. As a
> result, "our" "foobar" is still present after m_config_free() and can be
> freed by a simple free() (called "manually" in the output). After that, it
> finally is gone.

Not sure what exactly that was meant to show, I can't fully interpret
the output there.
However note that the values are stored twice: once in the program-accessible
variable (vo_winname here I guess) and once in the "slot data".
The slot data is there to allow "pushing" and "poping" configuration.
The reason is the file-specific options. They mean you have multiple
configs stored at the same time: you have the global options, which 
you push down and then set the file-specific ones. Once the file
is played, you "pop" the previous configuration, thus restoring the
state before the file-specific options were applied.

> BTW, my original question concerning the GUI was: What is the right way to
> change a string option's value so that memory associated with the old value
> gets freed and the new one will be freed when m_config_free() is executed?
> (I was supposing m_option_save() or m_config_set_option() do the job.)

For the m_option_* function:
For strings all of copy, save and set are the same function,
however m_option_set should be semantically the most correct.
However that one is rather low-level and the m_config_set_option
one should be better to use.