commit_packed_refs(): report errors rather than dying

Report errors via a `struct strbuf *err` rather than by calling
`die()`. To enable this goal, change `write_packed_entry()` to report
errors via a return value and `errno` rather than dying.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 60f4fa5e7a5f22e9d47acd9600d85d8e4f09ec85..2810785efcfe2244847b87199fa332004607f0de 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1094,6 +1094,7 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
        struct ref_iterator *iter;
        int ok;
        struct ref_to_prune *refs_to_prune = NULL;
+       struct strbuf err = STRBUF_INIT;
 
        lock_packed_refs(refs->packed_ref_store, LOCK_DIE_ON_ERROR);
 
@@ -1128,10 +1129,11 @@ static int files_pack_refs(struct ref_store *ref_store, unsigned int flags)
        if (ok != ITER_DONE)
                die("error while iterating over references");
 
-       if (commit_packed_refs(refs->packed_ref_store))
-               die_errno("unable to overwrite old ref-pack file");
+       if (commit_packed_refs(refs->packed_ref_store, &err))
+               die("unable to overwrite old ref-pack file: %s", err.buf);
 
        prune_refs(refs, refs_to_prune);
+       strbuf_release(&err);
        return 0;
 }
 
@@ -2693,9 +2695,7 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,
                                       &update->new_oid);
        }
 
-       if (commit_packed_refs(refs->packed_ref_store)) {
-               strbuf_addf(err, "unable to commit packed-refs file: %s",
-                           strerror(errno));
+       if (commit_packed_refs(refs->packed_ref_store, err)) {
                ret = TRANSACTION_GENERIC_ERROR;
                goto cleanup;
        }

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 4676dc395954ff7873890af2709de982dcd9a3fa..18ce47fcb7b788941d36e65f32b333ce672a8765 100644 (file)
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -493,15 +493,19 @@ static struct ref_iterator *packed_ref_iterator_begin(
 
 /*
  * Write an entry to the packed-refs file for the specified refname.
- * If peeled is non-NULL, write it as the entry's peeled value.
+ * If peeled is non-NULL, write it as the entry's peeled value. On
+ * error, return a nonzero value and leave errno set at the value left
+ * by the failing call to `fprintf()`.
  */
-static void write_packed_entry(FILE *fh, const char *refname,
-                              const unsigned char *sha1,
-                              const unsigned char *peeled)
+static int write_packed_entry(FILE *fh, const char *refname,
+                             const unsigned char *sha1,
+                             const unsigned char *peeled)
 {
-       fprintf_or_die(fh, "%s %s\n", sha1_to_hex(sha1), refname);
-       if (peeled)
-               fprintf_or_die(fh, "^%s\n", sha1_to_hex(peeled));
+       if (fprintf(fh, "%s %s\n", sha1_to_hex(sha1), refname) < 0 ||
+           (peeled && fprintf(fh, "^%s\n", sha1_to_hex(peeled)) < 0))
+               return -1;
+
+       return 0;
 }
 
 int lock_packed_refs(struct ref_store *ref_store, int flags)
@@ -550,49 +554,74 @@ static const char PACKED_REFS_HEADER[] =
 /*
  * Write the current version of the packed refs cache from memory to
  * disk. The packed-refs file must already be locked for writing (see
- * lock_packed_refs()). Return zero on success. On errors, set errno
- * and return a nonzero value.
+ * lock_packed_refs()). Return zero on success. On errors, rollback
+ * the lockfile, write an error message to `err`, and return a nonzero
+ * value.
  */
-int commit_packed_refs(struct ref_store *ref_store)
+int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err)
 {
        struct packed_ref_store *refs =
                packed_downcast(ref_store, REF_STORE_WRITE | REF_STORE_MAIN,
                                "commit_packed_refs");
        struct packed_ref_cache *packed_ref_cache =
                get_packed_ref_cache(refs);
-       int ok, error = 0;
-       int save_errno = 0;
+       int ok;
+       int ret = -1;
        FILE *out;
        struct ref_iterator *iter;
 
        if (!is_lock_file_locked(&refs->lock))
-               die("BUG: packed-refs not locked");
+               die("BUG: commit_packed_refs() called when unlocked");
 
        out = fdopen_lock_file(&refs->lock, "w");
-       if (!out)
-               die_errno("unable to fdopen packed-refs descriptor");
+       if (!out) {
+               strbuf_addf(err, "unable to fdopen packed-refs tempfile: %s",
+                           strerror(errno));
+               goto error;
+       }
 
-       fprintf_or_die(out, "%s", PACKED_REFS_HEADER);
+       if (fprintf(out, "%s", PACKED_REFS_HEADER) < 0) {
+               strbuf_addf(err, "error writing to %s: %s",
+                           get_lock_file_path(&refs->lock), strerror(errno));
+               goto error;
+       }
 
        iter = cache_ref_iterator_begin(packed_ref_cache->cache, NULL, 0);
        while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
                struct object_id peeled;
                int peel_error = ref_iterator_peel(iter, &peeled);
 
-               write_packed_entry(out, iter->refname, iter->oid->hash,
-                                  peel_error ? NULL : peeled.hash);
+               if (write_packed_entry(out, iter->refname, iter->oid->hash,
+                                      peel_error ? NULL : peeled.hash)) {
+                       strbuf_addf(err, "error writing to %s: %s",
+                                   get_lock_file_path(&refs->lock),
+                                   strerror(errno));
+                       ref_iterator_abort(iter);
+                       goto error;
+               }
        }
 
-       if (ok != ITER_DONE)
-               die("error while iterating over references");
+       if (ok != ITER_DONE) {
+               strbuf_addf(err, "unable to write packed-refs file: "
+                           "error iterating over old contents");
+               goto error;
+       }
 
        if (commit_lock_file(&refs->lock)) {
-               save_errno = errno;
-               error = -1;
+               strbuf_addf(err, "error overwriting %s: %s",
+                           refs->path, strerror(errno));
+               goto out;
        }
+
+       ret = 0;
+       goto out;
+
+error:
+       rollback_lock_file(&refs->lock);
+
+out:
        release_packed_ref_cache(packed_ref_cache);
-       errno = save_errno;
-       return error;
+       return ret;
 }
 
 /*
@@ -628,7 +657,7 @@ int repack_without_refs(struct ref_store *ref_store,
                                "repack_without_refs");
        struct ref_dir *packed;
        struct string_list_item *refname;
-       int ret, needs_repacking = 0, removed = 0;
+       int needs_repacking = 0, removed = 0;
 
        packed_assert_main_repository(refs, "repack_without_refs");
        assert(err);
@@ -665,11 +694,7 @@ int repack_without_refs(struct ref_store *ref_store,
        }
 
        /* Write what remains */
-       ret = commit_packed_refs(&refs->base);
-       if (ret)
-               strbuf_addf(err, "unable to overwrite old ref-pack file: %s",
-                           strerror(errno));
-       return ret;
+       return commit_packed_refs(&refs->base, err);
 }
 
 static int packed_init_db(struct ref_store *ref_store, struct strbuf *err)

diff --git a/refs/packed-backend.h b/refs/packed-backend.h
index beea9c14b5cb93ce374e3f619d479b116a1108a5..3d4057b65b331fbb9fba2a5d9072b31417e411d4 100644 (file)
--- a/refs/packed-backend.h
+++ b/refs/packed-backend.h
@@ -14,7 +14,7 @@ int lock_packed_refs(struct ref_store *ref_store, int flags);
 void add_packed_ref(struct ref_store *ref_store,
                    const char *refname, const struct object_id *oid);
 
-int commit_packed_refs(struct ref_store *ref_store);
+int commit_packed_refs(struct ref_store *ref_store, struct strbuf *err);
 
 int repack_without_refs(struct ref_store *ref_store,
                        struct string_list *refnames, struct strbuf *err);