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

["Matthias Andree" <matthias.andree@gmx.de>]

Am 09.07.2009, 15:01 Uhr, schrieb Johannes Schindelin  
<Johannes.Schindelin@gmx.de>:

> Hi,
>
> On Thu, 9 Jul 2009, Thomas Rast wrote:
>
>> 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.
>
> Good point.  So we really need something like
>
> 	err = ferror(f);
> 	err |= fclose(f); /* call fclose() even if there was an error */
> 	if (err)
> 		error...

I've made such a patch, to appear soon on the list (sorry for not Cc:'ing  
it).

-- 
Matthias Andree