[FFmpeg-devel] [PATCH 00/13] check all fclose usage

[Ganesh Ajjanagadde]

On Tue, Jan 12, 2016 at 4:38 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Mon, 11 Jan 2016 23:25:02 -0500
> Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>
>> Some preliminary work has already been done on fclose checking. This completes
>> the work, modulo a few exceptions:
>
> 1. Printing warnings is completely useless unless maybe in ffmpeg.c
> interactive usage (if the user has good eyes). For API users or
> ffmpeg.c automated shell scripts this helps absolutely nothing.

Say a user does some work, runs his/her scripts that are automated,
and something happens (see e.g the lavfi psnr/ssim patch). They let
them run for a couple of days, but do not get what they expect due to
some e.g NFS error. They then look at their logs, everything seemed to
have proceeded fine, since the fclose was unchecked. See e.g
https://ffmpeg.org/ffmpeg-filters.html#psnr, look at the stats_file
option. Of course, the log file itself may not be written correctly,
but that does not mean we should avoid a "best effort".

Sure, we are not going as far as gnulib's close-stream
https://www.gnu.org/software/gnulib/coverage/gllib/close-stream.c.gcov.frameset.html,
but that does not mean we should not do some elementary checks for
files opened in write mode. On read mode, it is more subjective and
less important.

> 2. There are some "theoretical" checks for fclose() on files opened in
> read-only mode.

I have already written that in the respective patches, so there is no
need to repeat the obvious. I also still find it a good idea to log -
see below, but am open to further technical discussion.

> This makes no sense. Even if fclose() should fail for
> whatever obscure reasons there might be, reading already worked
> without errors, and thus closing failure has no meaning to use.

Well, reading may not have worked, and the fclose may have been called
in a failure path. FFmpeg varies here: e.g lavfi/vf_lut3d(587 or so)
actually logs at any stage if the reading fails, but in some cases
nothing gets logged whatsoever if the reading fails. On the other hand
see lavc/dvdsubdec(663 or so), some fread/fseek's, although checked,
have nothing logged in case of failure. Thus, a user has no idea
whether the file has been read correctly. Checking close at least
helps in such situations, though it may not be perfect here. Yes, one
could add messages for every fread/fseek, but this is a less verbose
method.

One could argue about WARNING vs DEBUG or whatever, but that is a
separate, more subjective issue.

>
> Just what is the point?

Can you please stop trolling patches with such gratuitous statements at the end?

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