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

[Reimar Döffinger]

On Fri, Aug 22, 2014 at 07:47:09AM +0200, Reimar Döffinger wrote:
> 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.

Pushed, with some minor refinements like bumping minor in case someone
really needs to test for it.