Re: [FFmpeg-devel] [PATCH] avformat: Add check for ff_get_extradata

Re: [FFmpeg-devel] [PATCH] avformat: Add check for ff_get_extradata

[Jiasheng Jiang]

On Tue, Feb 22, 2022 at 04:10:51PM +0800, Paul B Mahol wrote:

>> As the potential failure of the memory allocation, the ff_get_extradata()
>> could return error if fails.
>> Therefore, it should be better to deal with the return value of the
>> ff_get_extradata() and return error if fails.
>>
> 
> 
> Not really necessary. Does not fix anything.

It is true that the 'extradata' could be NULL and have not used.
But I have checked many other callers of the ff_get_extradata(),
such as avi_read_header() in `libavformat/avidec.c`.
They all have checked the return value to guarantee the 'extradata'
to be non-NULL.
That means in the future, if the 'aix->extradata' is used, the programmer
may not notice that 'aix->extradata' is especial and needs to be checked
before use.
Therefore, I think it is necessary to add the check to guarantee the
consisitency of the code.

Thanks,
Jiang

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat: Add check for ff_get_extradata

[Paul B Mahol]

On Tue, Feb 22, 2022 at 10:14 AM Jiasheng Jiang <jiasheng@iscas.ac.cn>
wrote:

> On Tue, Feb 22, 2022 at 04:10:51PM +0800, Paul B Mahol wrote:
>
> >> As the potential failure of the memory allocation, the
> ff_get_extradata()
> >> could return error if fails.
> >> Therefore, it should be better to deal with the return value of the
> >> ff_get_extradata() and return error if fails.
> >>
> >
> >
> > Not really necessary. Does not fix anything.
>
> It is true that the 'extradata' could be NULL and have not used.
> But I have checked many other callers of the ff_get_extradata(),
> such as avi_read_header() in `libavformat/avidec.c`.
> They all have checked the return value to guarantee the 'extradata'
> to be non-NULL.
> That means in the future, if the 'aix->extradata' is used, the programmer
> may not notice that 'aix->extradata' is especial and needs to be checked
> before use.
> Therefore, I think it is necessary to add the check to guarantee the
> consisitency of the code.
>

Nope, that does not make sense, it should always be checked.


>
> Thanks,
> Jiang
>
>
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat: Add check for ff_get_extradata

[Paul B Mahol]

On Tue, Feb 22, 2022 at 8:50 AM Jiasheng Jiang <jiasheng@iscas.ac.cn> wrote:

> As the potential failure of the memory allocation, the ff_get_extradata()
> could return error if fails.
> Therefore, it should be better to deal with the return value of the
> ff_get_extradata() and return error if fails.
>


Not really necessary. Does not fix anything.


>
> Fixes: 2d720069a9 ("avformat: add aix demuxer")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  libavformat/aixdec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/aixdec.c b/libavformat/aixdec.c
> index 59c3d60da3..89f73b6913 100644
> --- a/libavformat/aixdec.c
> +++ b/libavformat/aixdec.c
> @@ -40,7 +40,7 @@ static int aix_read_header(AVFormatContext *s)
>      unsigned segment_list_offset = 0x20;
>      unsigned segment_list_entry_size = 0x10;
>      unsigned size;
> -    int i;
> +    int i, ret;
>
>      avio_skip(s->pb, 4);
>      first_offset = avio_rb32(s->pb) + 8;
> @@ -77,7 +77,9 @@ static int aix_read_header(AVFormatContext *s)
>          if (size <= 8)
>              return AVERROR_INVALIDDATA;
>          avio_skip(s->pb, 8);
> -        ff_get_extradata(s, s->streams[i]->codecpar, s->pb, size - 8);
> +        ret = ff_get_extradata(s, s->streams[i]->codecpar, s->pb, size -
> 8);
> +        if (ret < 0)
> +            return ret;
>      }
>
>      return 0;
> --
> 2.25.1
>
> _______________________________________________
> 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".
>
_______________________________________________
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".

[FFmpeg-devel] [PATCH] avformat: Add check for ff_get_extradata

[Jiasheng Jiang]

As the potential failure of the memory allocation, the ff_get_extradata()
could return error if fails.
Therefore, it should be better to deal with the return value of the
ff_get_extradata() and return error if fails.

Fixes: 2d720069a9 ("avformat: add aix demuxer")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 libavformat/aixdec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/aixdec.c b/libavformat/aixdec.c
index 59c3d60da3..89f73b6913 100644
--- a/libavformat/aixdec.c
+++ b/libavformat/aixdec.c
@@ -40,7 +40,7 @@ static int aix_read_header(AVFormatContext *s)
     unsigned segment_list_offset = 0x20;
     unsigned segment_list_entry_size = 0x10;
     unsigned size;
-    int i;
+    int i, ret;
 
     avio_skip(s->pb, 4);
     first_offset = avio_rb32(s->pb) + 8;
@@ -77,7 +77,9 @@ static int aix_read_header(AVFormatContext *s)
         if (size <= 8)
             return AVERROR_INVALIDDATA;
         avio_skip(s->pb, 8);
-        ff_get_extradata(s, s->streams[i]->codecpar, s->pb, size - 8);
+        ret = ff_get_extradata(s, s->streams[i]->codecpar, s->pb, size - 8);
+        if (ret < 0)
+            return ret;
     }
 
     return 0;
-- 
2.25.1

_______________________________________________
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".