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

From: Thomas Rast <trast@student.ethz.ch>
Date: Jul 9, 2009, 11:24 UTC
Message-ID: <200907091324.17643.trast@student.ethz.ch>
In-Reply-To: <alpine.DEB.1.00.0907091302520.4339@intel-tinevez-2-302>

Johannes Schindelin wrote:

Show 15 quoted lines

> 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