[FFmpeg-devel] [PATCH 3/4] dict.c: Free non-strduped av_dict_set arguments on error.

[Reimar Döffinger]

On 22.08.2014, at 07:36, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> On 22.08.2014, at 04:06, Michael Niedermayer <michaelni at gmx.at> wrote:
>> On Sat, Aug 16, 2014 at 02:43:46PM +0200, Reimar Döffinger wrote:
>>> On Wed, Aug 13, 2014 at 01:57:56PM +0200, Michael Niedermayer wrote:
>>>> On Mon, Aug 11, 2014 at 09:17:18PM +0200, Reimar Döffinger wrote:
>>>>> Unfortunately this was not explicitly documented and thus
>>>>> might be very risky.
>>>> 
>>>> yes
>>>> 
>>>> maybe a AV_DICT_FREE_ARGS_ON_ERROR could be used, but its not as
>>>> convenient as getting the behavior by default
>>> 
>>> That would add a lot of clutter and make it more inconvenient to use
>>> and likely to be forgotten.
>>> I also honestly see no real use-case for not setting it.
>>> One way to solve this would be to deprecate AV_DICT_DONT_STRDUP_KEY/
>>> AV_DICT_DONT_STRDUP_VAL, warning that they fail to free on error
>>> and then add new values, e.g. AV_DICT_OWN_KEY (better suggestions?)
>>> with the new behaviour.
>>> Will still be a bit of a pain to replace all uses in FFmpeg, but
>>> manageable.
>>> What do you think? Better suggestions?
>> 
>> i guess its either to apply the patch or not, i dont see a better
>> solution. renaming and deprecating options is certainly a bigger mess
>> and doesnt avoid the problem 
> 
> Given how fixing all uses in FFmpeg would be a bigger mess IMHO I'd favour applying it, if necessary with the ABI bump as excuse.
> If you agree apply it (or wait for me to do it this evening at earliest).

Note I found only one use outside FFmpeg, in VLC's avcommon.h, and it does not check return value, so it would actually be fixed by this change, too.