Skip to content

Commit

Permalink
refs.c: add err arguments to reflog functions
Browse files Browse the repository at this point in the history 
Add an err argument to log_ref_setup that can explain the reason
for a failure. This then eliminates the need to manage errno through
this function since we can just add strerror(errno) to the err string
when meaningful. No callers relied on errno from this function for
anything else than the error message.

Also add err arguments to private functions write_ref_to_lockfile,
log_ref_write_1, commit_ref_update. This again eliminates the need to
manage errno in these functions.

Some error messages are slightly reordered.

Update of a patch by Ronnie Sahlberg.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
David Turner authored and Junio C Hamano committed Jul 21, 2015
1 parent 912bd49 commit a4c653d
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 60 deletions.
8 changes: 5 additions & 3 deletions builtin/checkout.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,16 +623,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
struct strbuf log_file = STRBUF_INIT;
int ret;
const char *ref_name;
struct strbuf err = STRBUF_INIT;

ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
temp = log_all_ref_updates;
log_all_ref_updates = 1;
ret = log_ref_setup(ref_name, &log_file);
ret = log_ref_setup(ref_name, &log_file, &err);
log_all_ref_updates = temp;
strbuf_release(&log_file);
if (ret) {
fprintf(stderr, _("Can not do reflog for '%s'\n"),
opts->new_orphan_branch);
fprintf(stderr, _("Can not do reflog for '%s': %s\n"),
opts->new_orphan_branch, err.buf);
strbuf_release(&err);
return;
}
}
Expand Down
129 changes: 74 additions & 55 deletions refs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2910,9 +2910,11 @@ static int rename_ref_available(const char *oldname, const char *newname)
return ret;
}

static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1);
static int write_ref_to_lockfile(struct ref_lock *lock,
const unsigned char *sha1, struct strbuf *err);
static int commit_ref_update(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg);
const unsigned char *sha1, const char *logmsg,
struct strbuf *err);

int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
{
Expand Down Expand Up @@ -2973,9 +2975,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
}
hashcpy(lock->old_oid.hash, orig_sha1);

if (write_ref_to_lockfile(lock, orig_sha1) ||
commit_ref_update(lock, orig_sha1, logmsg)) {
error("unable to write current sha1 into %s", newrefname);
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
commit_ref_update(lock, orig_sha1, logmsg, &err)) {
error("unable to write current sha1 into %s: %s", newrefname, err.buf);
strbuf_release(&err);
goto rollback;
}

Expand All @@ -2991,9 +2994,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms

flag = log_all_ref_updates;
log_all_ref_updates = 0;
if (write_ref_to_lockfile(lock, orig_sha1) ||
commit_ref_update(lock, orig_sha1, NULL))
error("unable to write current sha1 into %s", oldrefname);
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
commit_ref_update(lock, orig_sha1, NULL, &err)) {
error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
strbuf_release(&err);
}
log_all_ref_updates = flag;

rollbacklog:
Expand Down Expand Up @@ -3048,8 +3053,8 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
}

/* This function must set a meaningful errno on failure */
int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
/* This function will fill in *err and return -1 on failure */
int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err)
{
int logfd, oflags = O_APPEND | O_WRONLY;
char *logfile;
Expand All @@ -3064,9 +3069,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
starts_with(refname, "refs/notes/") ||
!strcmp(refname, "HEAD"))) {
if (safe_create_leading_directories(logfile) < 0) {
int save_errno = errno;
error("unable to create directory for %s", logfile);
errno = save_errno;
strbuf_addf(err, "unable to create directory for %s: "
"%s", logfile, strerror(errno));
return -1;
}
oflags |= O_CREAT;
Expand All @@ -3079,20 +3083,16 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)

if (errno == EISDIR) {
if (remove_empty_directories(logfile)) {
int save_errno = errno;
error("There are still logs under '%s'",
logfile);
errno = save_errno;
strbuf_addf(err, "There are still logs under "
"'%s'", logfile);
return -1;
}
logfd = open(logfile, oflags, 0666);
}

if (logfd < 0) {
int save_errno = errno;
error("Unable to append to %s: %s", logfile,
strerror(errno));
errno = save_errno;
strbuf_addf(err, "unable to append to %s: %s",
logfile, strerror(errno));
return -1;
}
}
Expand Down Expand Up @@ -3130,15 +3130,16 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,

static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg,
struct strbuf *sb_log_file)
struct strbuf *sb_log_file, struct strbuf *err)
{
int logfd, result, oflags = O_APPEND | O_WRONLY;
char *log_file;

if (log_all_ref_updates < 0)
log_all_ref_updates = !is_bare_repository();

result = log_ref_setup(refname, sb_log_file);
result = log_ref_setup(refname, sb_log_file, err);

if (result)
return result;
log_file = sb_log_file->buf;
Expand All @@ -3151,26 +3152,25 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
result = log_ref_write_fd(logfd, old_sha1, new_sha1,
git_committer_info(0), msg);
if (result) {
int save_errno = errno;
strbuf_addf(err, "unable to append to %s: %s", log_file,
strerror(errno));
close(logfd);
error("Unable to append to %s", log_file);
errno = save_errno;
return -1;
}
if (close(logfd)) {
int save_errno = errno;
error("Unable to append to %s", log_file);
errno = save_errno;
strbuf_addf(err, "unable to append to %s: %s", log_file,
strerror(errno));
return -1;
}
return 0;
}

static int log_ref_write(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg)
const unsigned char *new_sha1, const char *msg,
struct strbuf *err)
{
struct strbuf sb = STRBUF_INIT;
int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb);
int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb, err);
strbuf_release(&sb);
return ret;
}
Expand All @@ -3182,36 +3182,36 @@ int is_branch(const char *refname)

/*
* Write sha1 into the open lockfile, then close the lockfile. On
* errors, rollback the lockfile and set errno to reflect the problem.
* errors, rollback the lockfile, fill in *err and
* return -1.
*/
static int write_ref_to_lockfile(struct ref_lock *lock,
const unsigned char *sha1)
const unsigned char *sha1, struct strbuf *err)
{
static char term = '\n';
struct object *o;

o = parse_object(sha1);
if (!o) {
error("Trying to write ref %s with nonexistent object %s",
lock->ref_name, sha1_to_hex(sha1));
strbuf_addf(err,
"Trying to write ref %s with nonexistent object %s",
lock->ref_name, sha1_to_hex(sha1));
unlock_ref(lock);
errno = EINVAL;
return -1;
}
if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
error("Trying to write non-commit object %s to branch %s",
sha1_to_hex(sha1), lock->ref_name);
strbuf_addf(err,
"Trying to write non-commit object %s to branch %s",
sha1_to_hex(sha1), lock->ref_name);
unlock_ref(lock);
errno = EINVAL;
return -1;
}
if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock->lk->fd, &term, 1) != 1 ||
close_ref(lock) < 0) {
int save_errno = errno;
error("Couldn't write %s", lock->lk->filename.buf);
strbuf_addf(err,
"Couldn't write %s", lock->lk->filename.buf);
unlock_ref(lock);
errno = save_errno;
return -1;
}
return 0;
Expand All @@ -3223,12 +3223,17 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
* necessary, using the specified lockmsg (which can be NULL).
*/
static int commit_ref_update(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
const unsigned char *sha1, const char *logmsg,
struct strbuf *err)
{
clear_loose_ref_cache(&ref_cache);
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg) < 0 ||
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, err) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg) < 0)) {
log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg, err) < 0)) {
char *old_msg = strbuf_detach(err, NULL);
strbuf_addf(err, "Cannot update the ref '%s': %s",
lock->ref_name, old_msg);
free(old_msg);
unlock_ref(lock);
return -1;
}
Expand All @@ -3251,14 +3256,21 @@ static int commit_ref_update(struct ref_lock *lock,
head_ref = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
head_sha1, &head_flag);
if (head_ref && (head_flag & REF_ISSYMREF) &&
!strcmp(head_ref, lock->ref_name))
log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg);
!strcmp(head_ref, lock->ref_name)) {
struct strbuf log_err = STRBUF_INIT;
if (log_ref_write("HEAD", lock->old_oid.hash, sha1,
logmsg, &log_err)) {
error("%s", log_err.buf);
strbuf_release(&log_err);
}
}
}
if (commit_ref(lock)) {
error("Couldn't set %s", lock->ref_name);
unlock_ref(lock);
return -1;
}

unlock_ref(lock);
return 0;
}
Expand All @@ -3271,6 +3283,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
int fd, len, written;
char *git_HEAD = git_pathdup("%s", ref_target);
unsigned char old_sha1[20], new_sha1[20];
struct strbuf err = STRBUF_INIT;

if (logmsg && read_ref(ref_target, old_sha1))
hashclr(old_sha1);
Expand Down Expand Up @@ -3319,8 +3332,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
#ifndef NO_SYMLINK_HEAD
done:
#endif
if (logmsg && !read_ref(refs_heads_master, new_sha1))
log_ref_write(ref_target, old_sha1, new_sha1, logmsg);
if (logmsg && !read_ref(refs_heads_master, new_sha1) &&
log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err)) {
error("%s", err.buf);
strbuf_release(&err);
}

free(git_HEAD);
return 0;
Expand Down Expand Up @@ -3956,14 +3972,19 @@ int ref_transaction_commit(struct ref_transaction *transaction,
* value, so we don't need to write it.
*/
} else if (write_ref_to_lockfile(update->lock,
update->new_sha1)) {
update->new_sha1,
err)) {
char *write_err = strbuf_detach(err, NULL);

/*
* The lock was freed upon failure of
* write_ref_to_lockfile():
*/
update->lock = NULL;
strbuf_addf(err, "cannot update the ref '%s'.",
update->refname);
strbuf_addf(err,
"cannot update the ref '%s': %s",
update->refname, write_err);
free(write_err);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
} else {
Expand All @@ -3989,11 +4010,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,

if (update->flags & REF_NEEDS_COMMIT) {
if (commit_ref_update(update->lock,
update->new_sha1, update->msg)) {
update->new_sha1, update->msg, err)) {
/* freed by commit_ref_update(): */
update->lock = NULL;
strbuf_addf(err, "Cannot update the ref '%s'.",
update->refname);
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
} else {
Expand Down
4 changes: 2 additions & 2 deletions refs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
#define REF_NODEREF 0x01

/*
* Setup reflog before using. Set errno to something meaningful on failure.
* Setup reflog before using. Fill in err and return -1 on failure.
*/
int log_ref_setup(const char *refname, struct strbuf *logfile);
int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err);

/** Reads log for the value of ref during at_time. **/
extern int read_ref_at(const char *refname, unsigned int flags,
Expand Down

0 comments on commit a4c653d

Please sign in to comment.