git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Brandon Casey <casey@nrlssc.navy.mil>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: [PATCH] fast-import.c: don't try to commit marks file if write failed
Date: Thu, 17 Jan 2008 10:58:34 -0600	[thread overview]
Message-ID: <478F893A.6050105@nrlssc.navy.mil> (raw)
In-Reply-To: <7vk5m9kvf7.fsf@gitster.siamese.dyndns.org>

We also move the assignment of -1 to the lock file descriptor
up, so that rollback_lock_file() can be called safely after a
possible attempt to fclose(). This matches the contents of
the 'if' statement just above testing success of fdopen().

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Since we forget the lock file descriptor there is a chance
that we will leave the file open if a write error occurs. We'll
still delete the file. I don't think it's worth bending
over backwards to make sure the file is closed on this failure.

-brandon


 fast-import.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 3609c24..4cf5092 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1545,19 +1545,27 @@ static void dump_marks(void)
 		return;
 	}
 
-	dump_marks_helper(f, 0, marks);
-	if (ferror(f) || fclose(f))
-		failure |= error("Unable to write marks file %s: %s",
-			mark_file, strerror(errno));
 	/*
-	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
-	 * assign -1 to the lock file descriptor so that commit_lock_file()
+	 * Since the lock file was fdopen()'ed, it should not be close()'ed.
+	 * Assign -1 to the lock file descriptor so that commit_lock_file()
 	 * won't try to close() it.
 	 */
 	mark_lock.fd = -1;
-	if (commit_lock_file(&mark_lock))
-		failure |= error("Unable to write commit file %s: %s",
+
+	dump_marks_helper(f, 0, marks);
+	if (ferror(f) || fclose(f)) {
+		rollback_lock_file(&mark_lock);
+		failure |= error("Unable to write marks file %s: %s",
 			mark_file, strerror(errno));
+		return;
+	}
+
+	if (commit_lock_file(&mark_lock)) {
+		rollback_lock_file(&mark_lock);
+		failure |= error("Unable to commit marks file %s: %s",
+			mark_file, strerror(errno));
+		return;
+	}
 }
 
 static int read_next_command(void)
-- 
1.5.4.rc3.17.gb63a4

  parent reply	other threads:[~2008-01-17 17:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-17  3:01 What's in git.git (stable frozen) Junio C Hamano
2008-01-17 12:56 ` Johannes Sixt
2008-01-17 16:58 ` Brandon Casey [this message]
2008-01-17 17:57   ` [PATCH] fast-import.c: don't try to commit marks file if write failed Junio C Hamano
2008-01-18  2:09     ` Shawn O. Pearce
2008-01-17 17:17 ` What's in git.git (stable frozen) Brandon Casey
2008-01-17 17:57   ` Junio C Hamano
2008-01-18  3:01 ` Shawn O. Pearce

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=478F893A.6050105@nrlssc.navy.mil \
    --to=casey@nrlssc.navy.mil \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).