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@xxxxxxxxxxxx>
---
refs/files-backend.c | 10 +++---
refs/packed-backend.c | 85 +++++++++++++++++++++++++++++++++------------------
refs/packed-backend.h | 2 +-
3 files changed, 61 insertions(+), 36 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 039d0343cb..4323394a52 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1082,6 +1082,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);
@@ -1116,10 +1117,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;
}
@@ -2681,9 +2683,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 5dbe4e4e59..5bee49d497 100644
--- 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 beea9c14b5..3d4057b65b 100644
--- 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);
--
2.11.0