[FFmpeg-devel] [PATCH] ffmpeg: check fclose return values

[Ganesh Ajjanagadde]

On Thu, Jan 7, 2016 at 3:57 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Thu, Jan 7, 2016 at 2:18 PM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
>> On Thu, Jan 07, 2016 at 11:16:27PM +0100, Michael Niedermayer wrote:
>>> On Thu, Jan 07, 2016 at 10:00:47AM -0800, Ganesh Ajjanagadde wrote:
>>> > On Thu, Jan 7, 2016 at 9:27 AM, Michael Niedermayer
>>> > <michael at niedermayer.cc> wrote:
>>> > > On Wed, Jan 06, 2016 at 09:00:46PM -0800, Ganesh Ajjanagadde wrote:
>>> > >> In the spirit of commit a956840cbc. Simple method to reproduce:
>>> > >> pass -vstats_file /dev/full to ffmpeg.
>>> > >>
>>> > >> All raw fclose usages in ffmpeg.c taken care of here.
>>> > >>
>>> > >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>> > >> ---
>>> > >>  ffmpeg.c | 13 ++++++++++---
>>> > >>  1 file changed, 10 insertions(+), 3 deletions(-)
>>> > >
>>> > > LGTM
>>> > >
>>> > > thanks
>>> >
>>> > So there is actually a problem with the diagnostic obtained, a more
>>> > accurate diagnostic is via errno, say strerror(errno) instead of
>>> > av_err2str(ret).
>>> > Comparison:
>>> > Error closing vstats file, loss of information possible: Operation not permitted
>>> > vs
>>> > Error closing vstats file, loss of information possible: No space left on device
>>> > for the provided /dev/full example.
>>> >
>>> > So there are a number of possiblities:
>>> > 1. Have 2 separate av_log lines, one for each of these.
>>> > 2. A single av_log line, using strerror(errno).
>>> > 3. Leave as is.
>>> >
>>> > I prefer 2. Let me know your preference, and I will push later.
>>>
>>> yes agree, 2.
>>
>> probably should use av_err2str() instead of strerror() though
>
> I thought strerror was C89? Your idea unfortunately causes problems
> (suspect it is because appropriate error tag is missing):
> Error closing vstats file, loss of information possible: Error number
> 28 occurred.

Did some digging, using strerror should be fine, see 984-989 of
cmdutils.c for very similar usage.

>
>>
>> [...]
>>
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> What does censorship reveal? It reveals fear. -- Julian Assange
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>