On Mon, 11 Aug 2014 21:17:18 +0200
Reimar Döffinger <reimar.doeffin...@gmx.de> wrote:
> Unfortunately this was not explicitly documented and thus
> might be very risky.
> But basically all uses I saw in FFmpeg had a memleak in these
> cases.
It's the more convenient behavior, although on the other hand it feels
wrong to change the input data on error.
This makes me wonder, isn't AV_DICT_DONT_STRDUP_* too obscure and too
much of a microoptimization, that we have to risk retro-guessing these
subtle semantics?
(But the patch is probably OK, IMHO.)
>
> Signed-off-by: Reimar Döffinger <reimar.doeffin...@gmx.de>
> ---
> libavutil/dict.c | 9 +++++++--
> libavutil/dict.h | 2 ++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 9fdc6d6..f23b768 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -91,7 +91,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
> AVDictionaryEntry *tmp = av_realloc(m->elems,
> (m->count + 1) *
> sizeof(*m->elems));
> if (!tmp)
> - return AVERROR(ENOMEM);
> + goto err_out;
> m->elems = tmp;
> }
> if (value) {
> @@ -105,7 +105,7 @@ int av_dict_set(AVDictionary **pm, const char *key, const
> char *value,
> int len = strlen(oldval) + strlen(value) + 1;
> char *newval = av_mallocz(len);
> if (!newval)
> - return AVERROR(ENOMEM);
> + goto err_out;
> av_strlcat(newval, oldval, len);
> av_freep(&oldval);
> av_strlcat(newval, value, len);
> @@ -120,6 +120,11 @@ int av_dict_set(AVDictionary **pm, const char *key,
> const char *value,
> }
>
> return 0;
> +
> +err_out:
> + if (flags & AV_DICT_DONT_STRDUP_KEY) av_free(key);
> + if (flags & AV_DICT_DONT_STRDUP_VAL) av_free(value);
> + return AVERROR(ENOMEM);
> }
>
> int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value,
> diff --git a/libavutil/dict.h b/libavutil/dict.h
> index 06f1621..5e319fe 100644
> --- a/libavutil/dict.h
> +++ b/libavutil/dict.h
> @@ -115,6 +115,8 @@ int av_dict_count(const AVDictionary *m);
> /**
> * Set the given entry in *pm, overwriting an existing entry.
> *
> + * Note: On error non-av_strduped arguments will be freed.
> + *
That doesn't really explain anything. Suggestion: mention
AV_DICT_DONT_STRDUP_* explicitly, and mention that the corresponding
argument will always be freed, even on error.
> * @param pm pointer to a pointer to a dictionary struct. If *pm is NULL
> * a dictionary struct is allocated and put in *pm.
> * @param key entry key to add to *pm (will be av_strduped depending on
> flags)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
