git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthias Andree <matthias.andree@gmx.de>
To: git@vger.kernel.org
Cc: Matthias Andree <matthias.andree@gmx.de>
Subject: [PATCH] Fix export_marks() error handling.
Date: Thu,  9 Jul 2009 15:28:01 +0200	[thread overview]
Message-ID: <1247146081-4692-1-git-send-email-matthias.andree@gmx.de> (raw)
In-Reply-To: <alpine.DEB.1.00.0907091500420.4339@intel-tinevez-2-302>

- Don't leak one FILE * on error per export_marks() call. Found with
  cppcheck and reported by Martin Ettl.

- Abort the potentially long for(;idnums.size;) loop on write errors.

- Add a trailing full-stop to error message when fopen() fails.

Signed-off-by: Matthias Andree <matthias.andree@gmx.de>
---
 builtin-fast-export.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 9a8a6fc..6c0956d 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -428,21 +428,30 @@ static void export_marks(char *file)
 	uint32_t mark;
 	struct object_decoration *deco = idnums.hash;
 	FILE *f;
+	int e;
 
 	f = fopen(file, "w");
 	if (!f)
-		error("Unable to open marks file %s for writing", file);
+		error("Unable to open marks file %s for writing.", file);
 
 	for (i = 0; i < idnums.size; i++) {
 		if (deco->base && deco->base->type == 1) {
 			mark = ptr_to_mark(deco->decoration);
-			fprintf(f, ":%"PRIu32" %s\n", mark,
+			e = fprintf(f, ":%"PRIu32" %s\n", mark,
 				sha1_to_hex(deco->base->sha1));
+			if (e < 0) break;
 		}
 		deco++;
 	}
 
-	if (ferror(f) || fclose(f))
+	/* do not optimize the next two lines - they must both be executed in
+	 * this order. || might short-circuit the fclose(), and combining them
+	 * into one statement might reverse the order of execution.
+	 * Also, fflush() may not be sufficient - on some file systems, the
+	 * error is still delayed until the final [f]close().  */
+	e  = ferror(f);
+	e |= fclose(f);
+	if (e)
 		error("Unable to write marks file %s.", file);
 }
 
-- 
1.6.3.3.385.g60647

  reply	other threads:[~2009-07-09 13:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-09  7:57 found a resource leak in file builtin-fast-export.c Martin Ettl
2009-07-09  8:31 ` Thomas Rast
2009-07-09 11:04   ` Johannes Schindelin
2009-07-09 11:24     ` Thomas Rast
2009-07-09 11:30       ` Andreas Ericsson
2009-07-09 13:01       ` Johannes Schindelin
2009-07-09 13:28         ` Matthias Andree [this message]
2009-07-11  9:45           ` [PATCH] Fix export_marks() error handling Stephen R. van den Berg
2009-07-13  8:01             ` Matthias Andree
2009-07-09 13:36         ` found a resource leak in file builtin-fast-export.c Matthias Andree
     [not found] <7v7hxyyfcg.fsf@alter.siamese.dyndns.org>
2009-07-24  8:17 ` [PATCH] Fix export_marks() error handling Matthias Andree
2009-07-24 16:35   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1247146081-4692-1-git-send-email-matthias.andree@gmx.de \
    --to=matthias.andree@gmx.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).