git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
MAMatthias Andree <matthias.andree@gmx.de>
Date
Jul 9, 2009, 13:36 UTC
Message-ID
<op.uwsyqnwt1e62zd@balu.cs.uni-paderborn.de>
In-Reply-To
<alpine.DEB.1.00.0907091500420.4339@intel-tinevez-2-302>

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

Show 36 quoted lines
> 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
Previous: Matthias Andree
Message 10 of 10 in “found a resource leak in file builtin-fast-export.c”
  1. Martin EttlJul 9, 2009
  2. Thomas RastJul 9, 2009
  3. Johannes SchindelinJul 9, 2009
  4. Thomas RastJul 9, 2009
  5. Andreas EricssonJul 9, 2009
  6. Johannes SchindelinJul 9, 2009
  7. Fix export_marks() error handling.Matthias Andree, Jul 9, 2009
  8. Stephen R. van den BergJul 11, 2009
  9. Matthias AndreeJul 13, 2009
  10. Matthias AndreeJul 9, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.