[FFmpeg-devel] [PATCH 1/2] avformat/utils: avoid unsigned integer overflows

[Andreas Rheinhardt]

Paul B Mahol:
> On 2/15/20, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
>> Marton Balint:
>>>
>>>
>>> On Sat, 15 Feb 2020, Nicolas George wrote:
>>>
>>>> Marton Balint (12020-02-15):
>>>>>> +    if (s->nb_streams)
>>>>>>     for (i = s->nb_streams - 1; i >= 0; i--)
>>>>> Maybe rewrite the loop instead?
>>>>>       for (i = s->nb_streams; i-- > 0;)
>>>>
>>>> Or
>>>>
>>>>    for (i = s->nb_streams - 1; i < s->nb_streams; i--)
>>>
>>> Or simply change the loops to normal ascending order? I don't see why
>>> these are descending loops in the first place.
>>>
>> Yeah. This sound like the best idea. One just has to call
>> free_stream() instead of ff_free_stream() for this to work.
> 
> But perhaps there is some reason for current order?

The reasons for this are historical: 85a57677 added ff_free_stream()
by factoring it out of avformat_free_context(). ff_free_stream() was
only intended to remove the last (highest index) stream of an
AVFormatContext and so the loop needed to be changed to counting down.
Meanwhile, Libav factored freeing a stream out, too, but differently
(in aeda1121): It added free_stream() (that simply frees without
updating the AVFormatContext (that is not among its parameters).
c03ffe17 refactored ff_free_stream() into what it is now, but it kept
using ff_free_stream() for freeing the stream and kept counting down
accordingly.

I couldn't find a reason why freeing the programs counts down. There
probably is none; anyway, we don't need history to know that one can
count upwards in both loops (when using free_stream() directly): We
can just read the code.

- Andreas