[FFmpeg-devel] [PATCH 11/15] avformat/flacdec:Return correct error-codes on read-failure

[Michael Niedermayer]

On Tue, Oct 29, 2024 at 03:50:05PM +0100, Tomas Härdin wrote:
> Reasonable enough. Might want a sample
> 
> Spotify comments
> ----------------
> Unexpected EOF is treated as invalid data
> 
> /Tomas

>  flacdec.c |   20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 8cf62c7b8b7e576cc0e2a0a1e49c4f42be5fc254  0011-avformat-flacdec-Return-correct-error-codes-on-read-.patch
> From 4d803c5f5c13e194a0e52d287f021b73ec7172bd Mon Sep 17 00:00:00 2001
> From: Ulrik <ulrikm at spotify.com>
> Date: Thu, 26 Jan 2023 17:51:02 +0100
> Subject: [PATCH 11/15] avformat/flacdec:Return correct error-codes on
>  read-failure
> 
> Forward errors from `avio_read` directly. When `avio_read` sees EOF before
> expected bytes can be read, consistently return `AVERROR_INVALIDDATA`
> 
> We used to return `AVERROR(AVERROR_INVALIDDATA)` when failing to read
> metadata block headers. `AVERROR_INVALIDDATA` is already negative, so
> wrapping in `AVERROR` leads to double-negation.
> 
> We used to return `AVERROR(EIO)` when failing to read extended metadata.
> However, many times, the IO-layer is not at fault, the input data is simply
> corrupted (truncated), so we return `AVERROR_INVALIDDATA` here as well.
> ---
>  libavformat/flacdec.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
> index 9f65c25864..be305fec82 100644
> --- a/libavformat/flacdec.c
> +++ b/libavformat/flacdec.c
> @@ -81,8 +81,15 @@ static int flac_read_header(AVFormatContext *s)
>  
>      /* process metadata blocks */
>      while (!avio_feof(s->pb) && !metadata_last) {
> -        if (avio_read(s->pb, header, 4) != 4)
> -            return AVERROR_INVALIDDATA;
> +        ret = avio_read(s->pb, header, 4);

> +        if (ret != 4) {
> +            if (ret < 0) {
> +                goto fail;
> +            } else {
> +                return AVERROR_INVALIDDATA;
> +            }
> +        }

this is wierd
because, one path goto fails the other returns directly.

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
than the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241029/2bc1e511/attachment.sig>