Subject: Re: found a resource leak in file builtin-fast-export.c

From: Thomas Rast <[email-masked]>
Date: Thu, 9 Jul 2009 13:24:14 +0200

Johannes Schindelin wrote:
> On Thu, 9 Jul 2009, Thomas Rast wrote:
> 
> > Martin Ettl wrote:
> > > -	if (ferror(f) || fclose(f))
> > > +	if (ferror(f))
> > >  		error("Unable to write marks file %s.", file);
> > > +  	fclose(f);
> > 
> > You no longer check the error returned by fclose().  This is
> > important, because the FILE* API may buffer writes, and a write error
> > may only become apparent when fclose() flushes the file.
> 
> Indeed.  A better fix would be to replace the || by a |, but this must be 
> accompanied by a comment so it does not get removed due to overzealous 
> compiler warnings.

Are you allowed to do that?  IIRC using | no longer guarantees that
ferror() is called before fclose(), and my local 'man 3p fclose' says
that

       After the call to fclose(), any use of stream results in
       undefined behavior.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch