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

Am 09.07.2009, 15:01 Uhr, schrieb Johannes Schindelin  
[off-list ref]:

Hi,

On Thu, 9 Jul 2009, Thomas Rast wrote:

quoted

Johannes Schindelin wrote:

quoted

On Thu, 9 Jul 2009, Thomas Rast wrote:

quoted

Martin Ettl wrote:

quoted

-	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

quoted

quoted

may only become apparent when fclose() flushes the file.

Indeed.  A better fix would be to replace the || by a |, but this  

must be

quoted

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