On Sun, Jan 26, 2025 at 9:41 PM Kacper Michajłow <kaspe...@gmail.com> wrote:
>
> Signed-off-by: Kacper Michajłow <kaspe...@gmail.com>
> ---
> libavformat/vqf.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/vqf.c b/libavformat/vqf.c
> index 58b1546f53..fbe54739cd 100644
> --- a/libavformat/vqf.c
> +++ b/libavformat/vqf.c
> @@ -66,10 +66,10 @@ static int add_metadata(AVFormatContext *s, uint32_t tag,
> return AVERROR(ENOMEM);
>
> ret = avio_read(s->pb, buf, len);
> - if (ret < 0)
> - return ret;
> - if (len != ret)
> - return AVERROR_INVALIDDATA;
> + if (ret < 0 || ret != len) {
> + av_free(buf);
> + return ret < 0 ? ret : AVERROR_INVALIDDATA;
> + }
> buf[len] = 0;
> AV_WL32(key, tag);
> return av_dict_set(&s->metadata, key, buf, AV_DICT_DONT_STRDUP_VAL);
LGTM, will apply.
In some other cases I would prefer `ret = AVERROR_INVALIDDATA; goto
error;` kind of handling where the freeing of `buf` is handled in a
separate block at the end where additional clean-up logic may be added
as required, but with the complexity of this function this is quite
fine like this. As for `av_freep` vs `av_free`, the `buf` variable
being a local pointer on the stack with the freeing directly going to
a return statement, given the scope of the function it feels OK that
the plain free function is utilized here.
Best regards,
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
