Skip to content

Commit a4c653d

Browse files
dturner-twgitster
dturner-tw
authored and
gitster
committed
refs.c: add err arguments to reflog functions
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>
1 parent 912bd49 commit a4c653d

File tree

3 files changed

+81
-60
lines changed

3 files changed

+81
-60
lines changed

‎builtin/checkout.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -623,16 +623,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
623623
struct strbuf log_file = STRBUF_INIT;
624624
int ret;
625625
const char *ref_name;
626+
struct strbuf err = STRBUF_INIT;
626627

627628
ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
628629
temp = log_all_ref_updates;
629630
log_all_ref_updates = 1;
630-
ret = log_ref_setup(ref_name, &log_file);
631+
ret = log_ref_setup(ref_name, &log_file, &err);
631632
log_all_ref_updates = temp;
632633
strbuf_release(&log_file);
633634
if (ret) {
634-
fprintf(stderr, _("Can not do reflog for '%s'\n"),
635-
opts->new_orphan_branch);
635+
fprintf(stderr, _("Can not do reflog for '%s': %s\n"),
636+
opts->new_orphan_branch, err.buf);
637+
strbuf_release(&err);
636638
return;
637639
}
638640
}

‎refs.c‎

Lines changed: 74 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2910,9 +2910,11 @@ static int rename_ref_available(const char *oldname, const char *newname)
29102910
return ret;
29112911
}
29122912

2913-
static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1);
2913+
static int write_ref_to_lockfile(struct ref_lock *lock,
2914+
const unsigned char *sha1, struct strbuf *err);
29142915
static int commit_ref_update(struct ref_lock *lock,
2915-
const unsigned char *sha1, const char *logmsg);
2916+
const unsigned char *sha1, const char *logmsg,
2917+
struct strbuf *err);
29162918

29172919
int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
29182920
{
@@ -2973,9 +2975,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
29732975
}
29742976
hashcpy(lock->old_oid.hash, orig_sha1);
29752977

2976-
if (write_ref_to_lockfile(lock, orig_sha1) ||
2977-
commit_ref_update(lock, orig_sha1, logmsg)) {
2978-
error("unable to write current sha1 into %s", newrefname);
2978+
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
2979+
commit_ref_update(lock, orig_sha1, logmsg, &err)) {
2980+
error("unable to write current sha1 into %s: %s", newrefname, err.buf);
2981+
strbuf_release(&err);
29792982
goto rollback;
29802983
}
29812984

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

29922995
flag = log_all_ref_updates;
29932996
log_all_ref_updates = 0;
2994-
if (write_ref_to_lockfile(lock, orig_sha1) ||
2995-
commit_ref_update(lock, orig_sha1, NULL))
2996-
error("unable to write current sha1 into %s", oldrefname);
2997+
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
2998+
commit_ref_update(lock, orig_sha1, NULL, &err)) {
2999+
error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
3000+
strbuf_release(&err);
3001+
}
29973002
log_all_ref_updates = flag;
29983003

29993004
rollbacklog:
@@ -3048,8 +3053,8 @@ static int copy_msg(char *buf, const char *msg)
30483053
return cp - buf;
30493054
}
30503055

3051-
/* This function must set a meaningful errno on failure */
3052-
int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
3056+
/* This function will fill in *err and return -1 on failure */
3057+
int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err)
30533058
{
30543059
int logfd, oflags = O_APPEND | O_WRONLY;
30553060
char *logfile;
@@ -3064,9 +3069,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
30643069
starts_with(refname, "refs/notes/") ||
30653070
!strcmp(refname, "HEAD"))) {
30663071
if (safe_create_leading_directories(logfile) < 0) {
3067-
int save_errno = errno;
3068-
error("unable to create directory for %s", logfile);
3069-
errno = save_errno;
3072+
strbuf_addf(err, "unable to create directory for %s: "
3073+
"%s", logfile, strerror(errno));
30703074
return -1;
30713075
}
30723076
oflags |= O_CREAT;
@@ -3079,20 +3083,16 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
30793083

30803084
if (errno == EISDIR) {
30813085
if (remove_empty_directories(logfile)) {
3082-
int save_errno = errno;
3083-
error("There are still logs under '%s'",
3084-
logfile);
3085-
errno = save_errno;
3086+
strbuf_addf(err, "There are still logs under "
3087+
"'%s'", logfile);
30863088
return -1;
30873089
}
30883090
logfd = open(logfile, oflags, 0666);
30893091
}
30903092

30913093
if (logfd < 0) {
3092-
int save_errno = errno;
3093-
error("Unable to append to %s: %s", logfile,
3094-
strerror(errno));
3095-
errno = save_errno;
3094+
strbuf_addf(err, "unable to append to %s: %s",
3095+
logfile, strerror(errno));
30963096
return -1;
30973097
}
30983098
}
@@ -3130,15 +3130,16 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
31303130

31313131
static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
31323132
const unsigned char *new_sha1, const char *msg,
3133-
struct strbuf *sb_log_file)
3133+
struct strbuf *sb_log_file, struct strbuf *err)
31343134
{
31353135
int logfd, result, oflags = O_APPEND | O_WRONLY;
31363136
char *log_file;
31373137

31383138
if (log_all_ref_updates < 0)
31393139
log_all_ref_updates = !is_bare_repository();
31403140

3141-
result = log_ref_setup(refname, sb_log_file);
3141+
result = log_ref_setup(refname, sb_log_file, err);
3142+
31423143
if (result)
31433144
return result;
31443145
log_file = sb_log_file->buf;
@@ -3151,26 +3152,25 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
31513152
result = log_ref_write_fd(logfd, old_sha1, new_sha1,
31523153
git_committer_info(0), msg);
31533154
if (result) {
3154-
int save_errno = errno;
3155+
strbuf_addf(err, "unable to append to %s: %s", log_file,
3156+
strerror(errno));
31553157
close(logfd);
3156-
error("Unable to append to %s", log_file);
3157-
errno = save_errno;
31583158
return -1;
31593159
}
31603160
if (close(logfd)) {
3161-
int save_errno = errno;
3162-
error("Unable to append to %s", log_file);
3163-
errno = save_errno;
3161+
strbuf_addf(err, "unable to append to %s: %s", log_file,
3162+
strerror(errno));
31643163
return -1;
31653164
}
31663165
return 0;
31673166
}
31683167

31693168
static int log_ref_write(const char *refname, const unsigned char *old_sha1,
3170-
const unsigned char *new_sha1, const char *msg)
3169+
const unsigned char *new_sha1, const char *msg,
3170+
struct strbuf *err)
31713171
{
31723172
struct strbuf sb = STRBUF_INIT;
3173-
int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb);
3173+
int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb, err);
31743174
strbuf_release(&sb);
31753175
return ret;
31763176
}
@@ -3182,36 +3182,36 @@ int is_branch(const char *refname)
31823182

31833183
/*
31843184
* Write sha1 into the open lockfile, then close the lockfile. On
3185-
* errors, rollback the lockfile and set errno to reflect the problem.
3185+
* errors, rollback the lockfile, fill in *err and
3186+
* return -1.
31863187
*/
31873188
static int write_ref_to_lockfile(struct ref_lock *lock,
3188-
const unsigned char *sha1)
3189+
const unsigned char *sha1, struct strbuf *err)
31893190
{
31903191
static char term = '\n';
31913192
struct object *o;
31923193

31933194
o = parse_object(sha1);
31943195
if (!o) {
3195-
error("Trying to write ref %s with nonexistent object %s",
3196-
lock->ref_name, sha1_to_hex(sha1));
3196+
strbuf_addf(err,
3197+
"Trying to write ref %s with nonexistent object %s",
3198+
lock->ref_name, sha1_to_hex(sha1));
31973199
unlock_ref(lock);
3198-
errno = EINVAL;
31993200
return -1;
32003201
}
32013202
if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
3202-
error("Trying to write non-commit object %s to branch %s",
3203-
sha1_to_hex(sha1), lock->ref_name);
3203+
strbuf_addf(err,
3204+
"Trying to write non-commit object %s to branch %s",
3205+
sha1_to_hex(sha1), lock->ref_name);
32043206
unlock_ref(lock);
3205-
errno = EINVAL;
32063207
return -1;
32073208
}
32083209
if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
32093210
write_in_full(lock->lk->fd, &term, 1) != 1 ||
32103211
close_ref(lock) < 0) {
3211-
int save_errno = errno;
3212-
error("Couldn't write %s", lock->lk->filename.buf);
3212+
strbuf_addf(err,
3213+
"Couldn't write %s", lock->lk->filename.buf);
32133214
unlock_ref(lock);
3214-
errno = save_errno;
32153215
return -1;
32163216
}
32173217
return 0;
@@ -3223,12 +3223,17 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
32233223
* necessary, using the specified lockmsg (which can be NULL).
32243224
*/
32253225
static int commit_ref_update(struct ref_lock *lock,
3226-
const unsigned char *sha1, const char *logmsg)
3226+
const unsigned char *sha1, const char *logmsg,
3227+
struct strbuf *err)
32273228
{
32283229
clear_loose_ref_cache(&ref_cache);
3229-
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg) < 0 ||
3230+
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, err) < 0 ||
32303231
(strcmp(lock->ref_name, lock->orig_ref_name) &&
3231-
log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg) < 0)) {
3232+
log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg, err) < 0)) {
3233+
char *old_msg = strbuf_detach(err, NULL);
3234+
strbuf_addf(err, "Cannot update the ref '%s': %s",
3235+
lock->ref_name, old_msg);
3236+
free(old_msg);
32323237
unlock_ref(lock);
32333238
return -1;
32343239
}
@@ -3251,14 +3256,21 @@ static int commit_ref_update(struct ref_lock *lock,
32513256
head_ref = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
32523257
head_sha1, &head_flag);
32533258
if (head_ref && (head_flag & REF_ISSYMREF) &&
3254-
!strcmp(head_ref, lock->ref_name))
3255-
log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg);
3259+
!strcmp(head_ref, lock->ref_name)) {
3260+
struct strbuf log_err = STRBUF_INIT;
3261+
if (log_ref_write("HEAD", lock->old_oid.hash, sha1,
3262+
logmsg, &log_err)) {
3263+
error("%s", log_err.buf);
3264+
strbuf_release(&log_err);
3265+
}
3266+
}
32563267
}
32573268
if (commit_ref(lock)) {
32583269
error("Couldn't set %s", lock->ref_name);
32593270
unlock_ref(lock);
32603271
return -1;
32613272
}
3273+
32623274
unlock_ref(lock);
32633275
return 0;
32643276
}
@@ -3271,6 +3283,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
32713283
int fd, len, written;
32723284
char *git_HEAD = git_pathdup("%s", ref_target);
32733285
unsigned char old_sha1[20], new_sha1[20];
3286+
struct strbuf err = STRBUF_INIT;
32743287

32753288
if (logmsg && read_ref(ref_target, old_sha1))
32763289
hashclr(old_sha1);
@@ -3319,8 +3332,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
33193332
#ifndef NO_SYMLINK_HEAD
33203333
done:
33213334
#endif
3322-
if (logmsg && !read_ref(refs_heads_master, new_sha1))
3323-
log_ref_write(ref_target, old_sha1, new_sha1, logmsg);
3335+
if (logmsg && !read_ref(refs_heads_master, new_sha1) &&
3336+
log_ref_write(ref_target, old_sha1, new_sha1, logmsg, &err)) {
3337+
error("%s", err.buf);
3338+
strbuf_release(&err);
3339+
}
33243340

33253341
free(git_HEAD);
33263342
return 0;
@@ -3956,14 +3972,19 @@ int ref_transaction_commit(struct ref_transaction *transaction,
39563972
* value, so we don't need to write it.
39573973
*/
39583974
} else if (write_ref_to_lockfile(update->lock,
3959-
update->new_sha1)) {
3975+
update->new_sha1,
3976+
err)) {
3977+
char *write_err = strbuf_detach(err, NULL);
3978+
39603979
/*
39613980
* The lock was freed upon failure of
39623981
* write_ref_to_lockfile():
39633982
*/
39643983
update->lock = NULL;
3965-
strbuf_addf(err, "cannot update the ref '%s'.",
3966-
update->refname);
3984+
strbuf_addf(err,
3985+
"cannot update the ref '%s': %s",
3986+
update->refname, write_err);
3987+
free(write_err);
39673988
ret = TRANSACTION_GENERIC_ERROR;
39683989
goto cleanup;
39693990
} else {
@@ -3989,11 +4010,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
39894010

39904011
if (update->flags & REF_NEEDS_COMMIT) {
39914012
if (commit_ref_update(update->lock,
3992-
update->new_sha1, update->msg)) {
4013+
update->new_sha1, update->msg, err)) {
39934014
/* freed by commit_ref_update(): */
39944015
update->lock = NULL;
3995-
strbuf_addf(err, "Cannot update the ref '%s'.",
3996-
update->refname);
39974016
ret = TRANSACTION_GENERIC_ERROR;
39984017
goto cleanup;
39994018
} else {

‎refs.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,9 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
189189
#define REF_NODEREF 0x01
190190

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

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

0 commit comments

Comments
 (0)