[FFmpeg-devel] [PATCH] ffmpeg: check return value of avio_closep for progress report

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 9:24 AM, James Almer <jamrial at gmail.com> wrote:
> On 1/5/2016 12:39 AM, Ganesh Ajjanagadde wrote:
>> avio_closep is not guaranteed to succeed, and its return value can
>> contain information regarding failure of preceding writes and silent
>> loss of data (man 2 close, man fclose). Users should know when the
>> progress was not successfully logged, and so a diagnostic is printed
>> here.
>>
>> The reason only one of these is changed in the patch is to get feedback:
>> a quick glance shows a large number of unchecked avio_close operations.
>> It may be tedious to check all of them, and relative importance varies
>> across instances. This was one which I feel is important.
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  ffmpeg.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index ee72f91..81f3697 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -1512,6 +1512,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
>>      static int64_t last_time = -1;
>>      static int qp_histogram[52];
>>      int hours, mins, secs, us;
>> +    int ret;
>>      float t;
>>
>>      if (!print_stats && !is_last_report && !progress_avio)
>> @@ -1678,7 +1679,9 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
>>          avio_flush(progress_avio);
>>          av_bprint_finalize(&buf_script, NULL);
>>          if (is_last_report) {
>> -            avio_closep(&progress_avio);
>> +            if ((ret = avio_closep(&progress_avio)) < 0)
>> +                av_log(NULL, AV_LOG_ERROR,
>> +                       "not possible to close progress log, loss of information possible: %s\n", av_err2str(ret));
>
> "Unable to close progress log" or "Error closing progress log" sounds better IMO.

Reworded and pushed, thanks. Now comes the tedious aspect: auditing
other such instances.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel