| Message ID | GV1P250MB07379DE5E17C55AF6A45A05C8F479@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | e867a29ec1eb71a5841e452a4f2bf946c096282a |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avutil/dict: Fix memleak when using AV_DICT_APPEND | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 9/13/22, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > When appending two values (due to AV_DICT_APPEND), the earlier code > would first zero-allocate a buffer of the required size and then > copy both parts into it via av_strlcat(). This is problematic, > as it leads to quadratic performance in case of frequent enlargements. > Fix this by using av_realloc() (which is hopefully designed to handle > such cases in a better way than simply throwing the buffer we already > have away) and by copying the string via memcpy() (after all, we already > calculated the strlen of both strings). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavutil/dict.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/libavutil/dict.c b/libavutil/dict.c > index c01165634e..1968063b0b 100644 > --- a/libavutil/dict.c > +++ b/libavutil/dict.c > @@ -98,16 +98,17 @@ int av_dict_set(AVDictionary **pm, const char *key, > const char *value, > return 0; > } > if (copy_value && flags & AV_DICT_APPEND) { > - size_t len = strlen(tag->value) + strlen(copy_value) + 1; > - char *newval = av_mallocz(len); > + size_t oldlen = strlen(tag->value); > + size_t new_part_len = strlen(copy_value); > + size_t len = oldlen + new_part_len + 1; > + char *newval = av_realloc(tag->value, len); > if (!newval) > goto err_out; > - av_strlcat(newval, tag->value, len); > - av_strlcat(newval, copy_value, len); > + memcpy(newval + oldlen, copy_value, new_part_len + 1); > av_freep(©_value); > copy_value = newval; > - } > - av_free(tag->value); > + } else > + av_free(tag->value); > av_free(tag->key); > *tag = m->elems[--m->count]; > } else if (copy_value) { > -- > 2.34.1 > LGTM > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavutil/dict.c b/libavutil/dict.c index c01165634e..1968063b0b 100644 --- a/libavutil/dict.c +++ b/libavutil/dict.c @@ -98,16 +98,17 @@ int av_dict_set(AVDictionary **pm, const char *key, const char *value, return 0; } if (copy_value && flags & AV_DICT_APPEND) { - size_t len = strlen(tag->value) + strlen(copy_value) + 1; - char *newval = av_mallocz(len); + size_t oldlen = strlen(tag->value); + size_t new_part_len = strlen(copy_value); + size_t len = oldlen + new_part_len + 1; + char *newval = av_realloc(tag->value, len); if (!newval) goto err_out; - av_strlcat(newval, tag->value, len); - av_strlcat(newval, copy_value, len); + memcpy(newval + oldlen, copy_value, new_part_len + 1); av_freep(©_value); copy_value = newval; - } - av_free(tag->value); + } else + av_free(tag->value); av_free(tag->key); *tag = m->elems[--m->count]; } else if (copy_value) {
When appending two values (due to AV_DICT_APPEND), the earlier code would first zero-allocate a buffer of the required size and then copy both parts into it via av_strlcat(). This is problematic, as it leads to quadratic performance in case of frequent enlargements. Fix this by using av_realloc() (which is hopefully designed to handle such cases in a better way than simply throwing the buffer we already have away) and by copying the string via memcpy() (after all, we already calculated the strlen of both strings). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavutil/dict.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)