[v12,2/8] object-file.c: do fsync() and close() before post-write die()

Commit Message

Ævar Arnfjörð Bjarmason March 29, 2022, 1:56 p.m. UTC

Change write_loose_object() to do an fsync() and close() before the
oideq() sanity check at the end. This change re-joins code that was
split up by the die() sanity check added in 748af44c63e (sha1_file: be
paranoid when creating loose objects, 2010-02-21).

I don't think that this change matters in itself, if we called die()
it was possible that our data wouldn't fully make it to disk, but in
any case we were writing data that we'd consider corrupted. It's
possible that a subsequent "git fsck" will be less confused now.

The real reason to make this change is that in a subsequent commit
we'll split this code in write_loose_object() into a utility function,
all its callers will want the preceding sanity checks, but not the
"oideq" check. By moving the close_loose_object() earlier it'll be
easier to reason about the introduction of the utility function.

Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
---
 object-file.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Patch

diff --git a/object-file.c b/object-file.c
index 62ebe236c90..5da458eccbf 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1886,7 +1886,14 @@  void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 	hash_object_file_literally(algo, buf, len, type_name(type), oid);
 }
 
-/* Finalize a file on disk, and close it. */
+/*
+ * We already did a write_buffer() to the "fd", let's fsync()
+ * and close().
+ *
+ * Finalize a file on disk, and close it. We might still die() on a
+ * subsequent sanity check, but let's not add to that confusion by not
+ * flushing any outstanding writes to disk first.
+ */
 static void close_loose_object(int fd)
 {
 	if (the_repository->objects->odb->will_destroy)
@@ -2006,12 +2013,12 @@  static int write_loose_object(const struct object_id *oid, char *hdr,
 		die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
 		    ret);
 	the_hash_algo->final_oid_fn(&parano_oid, &c);
+	close_loose_object(fd);
+
 	if (!oideq(oid, &parano_oid))
 		die(_("confused by unstable object source data for %s"),
 		    oid_to_hex(oid));
 
-	close_loose_object(fd);
-
 	if (mtime) {
 		struct utimbuf utb;
 		utb.actime = mtime;