[FFmpeg-devel] [FFmpeg-cvslog] avcodec/avpacket: Avoid unspecific return -1 for av_grow_packet()

[Carl Eugen Hoyos]

2019-01-07 3:23 GMT+01:00, Michael Niedermayer <michael at niedermayer.cc>:
> On Sun, Jan 06, 2019 at 10:43:53AM -0300, James Almer wrote:
>> On 1/6/2019 10:36 AM, Carl Eugen Hoyos wrote:
>> > 2019-01-02 0:23 GMT+01:00, Michael Niedermayer <git at videolan.org>:
>> >> ffmpeg | branch: master | Michael Niedermayer <michael at niedermayer.cc>
>> >> | Mon
>> >> Dec 31 18:25:18 2018 +0100| [9520d51e21f9aa5adc807b0b89322bd822b06738]
>> >> |
>> >> committer: Michael Niedermayer
>> >>
>> >> avcodec/avpacket: Avoid unspecific return -1 for av_grow_packet()
>> >>
>> >> Reviewed-by: Paul B Mahol <onemda at gmail.com>
>> >> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>> >>
>> >>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=9520d51e21f9aa5adc807b0b89322bd822b06738
>> >> ---
>> >>
>> >>  libavcodec/avpacket.c | 4 ++--
>> >>  1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> >> index e160ad3033..11ac4e80cd 100644
>> >> --- a/libavcodec/avpacket.c
>> >> +++ b/libavcodec/avpacket.c
>> >> @@ -112,7 +112,7 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>> >>      av_assert0((unsigned)pkt->size <= INT_MAX -
>> >> AV_INPUT_BUFFER_PADDING_SIZE);
>> >>      if ((unsigned)grow_by >
>> >>          INT_MAX - (pkt->size + AV_INPUT_BUFFER_PADDING_SIZE))
>> >> -        return -1;
>> >> +        return AVERROR(ENOMEM);
>> >>
>> >>      new_size = pkt->size + grow_by + AV_INPUT_BUFFER_PADDING_SIZE;
>> >>      if (pkt->buf) {
>> >> @@ -124,7 +124,7 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>> >>          } else {
>> >>              data_offset = pkt->data - pkt->buf->data;
>> >>              if (data_offset > INT_MAX - new_size)
>> >> -                return -1;
>> >> +                return AVERROR(ENOMEM);
>> >
>> > Is this really correct?
>> > At least on some 64bit systems, larger allocations should be possible,
>> > we are simply assuming invalid data if such an allocation is tried, no?
>>
>> Even if av_max_alloc() allows the API user to allocate more than INT_MAX
>> bytes, AVPackets can't handle it as they use int for size.
>
>>
>> And afaik, we have been using ERANGE as return error when an attempted
>> allocation is outside the allowed range. So maybe that's what should be
>> used here.
>
> about error codes
> allocation can fail for many reasons
> for example lack of physical memory, per process or per allocation
> limits, or others.

True, we also use ENOMEM in mov.c

Carl Eugen