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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.


A more important question; Do we really care? I haven't looked closely
at the code, but afair the marks file is written once per invocation,
so leaking its file descriptor sounds like something we won't really
bother about.


--
Andreas Ericsson                   andreas.ericsson@xxxxxx
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]