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

[wm4]

On Wed, 13 Jan 2016 10:48:01 -0500
Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:

> On Wed, Jan 13, 2016 at 4:05 AM, wm4 <nfxjfg at googlemail.com> wrote:
> > On Tue, 12 Jan 2016 10:07:08 -0500
> > Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> >  
> >> On Tue, Jan 12, 2016 at 9:43 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:  
> >> > Hi,
> >> >
> >> > On Tue, Jan 12, 2016 at 7:52 AM, Ganesh Ajjanagadde <gajjanag at mit.edu>
> >> > wrote:
> >> >  
> >> >> On Tue, Jan 12, 2016 at 4:38 AM, wm4 <nfxjfg at googlemail.com> wrote:  
> >> >> > 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.  
> >> >
> >> >
> >> > Then the error should be in the code path of fread(), not fclose().
> >> > Displaying an error (in whatever way) related to close while the actual
> >> > problem was reading data is going to be confusing to the user.  
> >>
> >> Read the rest of it; checking for every fread/fseek is not productive;
> >> there are at least 3 of fread/fseek in the example I gave. Printing at
> >> the time of closure is a natural means of doing things, again see:
> >> https://www.gnu.org/ghm/2011/paris/slides/jim-meyering-goodbye-world.pdf
> >> (particularly slide 7).
> >>
> >> Again, this is more important for write than read. Or put in other  
> >
> > Indeed, the slides don't mention reading AT ALL.
> >
> > A quick grep shows that we apparently do at least _something_ with the
> > return value of every single fread() call, and most if not all are in
> > test/example code.
> >  
> >> words: if the approach of checking at close is going to be nacked no
> >> matter what the contents of the message is; I will wash my hands off
> >> this issue wrt files opened read-only.
> >>  
> >> >  
> >> >> Just what is the point?
> >> >>
> >> >> Can you please stop trolling patches with such gratuitous statements at
> >> >> the end?  
> >
> > Isn't my question justified?  
> 
> Your first point was not:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2016-January/186973.html. It
> was simply gratuituous flame bait. Keep up the good work of
> selectively replying.

Both of these are perfectly valid points. To express the first in a
completely different way: should scripting/API users be forced to
parse FFmpeg log message to catch failures? Depending on the answer,
your patches are probably good enough, so don't mind me.

> > I guess I'll
> > concentrate on doing actual work instead,  
> 
> And you consider mpv "actual work". Not everyone may agree with such
> an assessment. I certainly do not. Minds infinitely greater than
> yours, mine, or anyone else's here do "actual work". Even on a
> relative scale, I don't care about a wastage of human effort on
> duplicating media playback, something VLC/mplayer/mplayer2/ffplay and
> goodness knows what does.

Oh no, I'm using FFmepg for something! I should be fucking ashamed. My
perspective might be somewhat locked-in to the software that I'm
maintaining and that happens to rely a lot on FFmpeg, but I don't think
it's as bad as you're trying to put all the time. Also I never implied
"mpv" when I wrote "actual work" above. Why are you focusing so much on
it?

Plus, I'm not the only one who works on software that uses FFmpeg.
There are many developers who use FFmpeg or its libraries selfishly for
their very own projects, and whose contributions or comments are
sometimes very specific to their own needs, yet they never get accused
of it.