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>

David Turner committed Jul 21, 2015 at 17:04 UTC a4c653dfcd05c987028b847092a1ee7e5d86a596
3 files changed +81 -60
builtin/checkout.c
+5 -3
@@ -623,16 +623,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
623 struct strbuf log_file = STRBUF_INIT;
624 int ret;
625 const char *ref_name;
626 + struct strbuf err = STRBUF_INIT;
627
628 ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
629 temp = log_all_ref_updates;
630 log_all_ref_updates = 1;
630 - ret = log_ref_setup(ref_name, &log_file);
631 + ret = log_ref_setup(ref_name, &log_file, &err);
632 log_all_ref_updates = temp;
633 strbuf_release(&log_file);
634 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);
638 return;
639 }
640 }
refs.c
+74 -55
@@ -2910,9 +2910,11 @@ static int rename_ref_available(const char *oldname, const char *newname)
2910 return ret;
2911 }
2912
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);
2915 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);
2918
2919 int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
2920 {
@@ -2973,9 +2975,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
2975 }
2976 hashcpy(lock->old_oid.hash, orig_sha1);
2977
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);
2982 goto rollback;
2983 }
2984
@@ -2991,9 +2994,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
2994
2995 flag = log_all_ref_updates;
2996 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 + }
3002 log_all_ref_updates = flag;
3003
3004 rollbacklog:
@@ -3048,8 +3053,8 @@ static int copy_msg(char *buf, const char *msg)
3053 return cp - buf;
3054 }
3055
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)
3058 {
3059 int logfd, oflags = O_APPEND | O_WRONLY;
3060 char *logfile;
@@ -3064,9 +3069,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
3069 starts_with(refname, "refs/notes/") ||
3070 !strcmp(refname, "HEAD"))) {
3071 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));
3074 return -1;
3075 }
3076 oflags |= O_CREAT;
@@ -3079,20 +3083,16 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
3083
3084 if (errno == EISDIR) {
3085 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);
3088 return -1;
3089 }
3090 logfd = open(logfile, oflags, 0666);
3091 }
3092
3093 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));
3096 return -1;
3097 }
3098 }
@@ -3130,7 +3130,7 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
3130
3131 static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
3132 const unsigned char *new_sha1, const char *msg,
3133 - struct strbuf *sb_log_file)
3133 + struct strbuf *sb_log_file, struct strbuf *err)
3134 {
3135 int logfd, result, oflags = O_APPEND | O_WRONLY;
3136 char *log_file;
@@ -3138,7 +3138,8 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
3138 if (log_all_ref_updates < 0)
3139 log_all_ref_updates = !is_bare_repository();
3140
3141 - result = log_ref_setup(refname, sb_log_file);
3141 + result = log_ref_setup(refname, sb_log_file, err);
3142 +
3143 if (result)
3144 return result;
3145 log_file = sb_log_file->buf;
@@ -3151,26 +3152,25 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
3152 result = log_ref_write_fd(logfd, old_sha1, new_sha1,
3153 git_committer_info(0), msg);
3154 if (result) {
3154 - int save_errno = errno;
3155 + strbuf_addf(err, "unable to append to %s: %s", log_file,
3156 + strerror(errno));
3157 close(logfd);
3156 - error("Unable to append to %s", log_file);
3157 - errno = save_errno;
3158 return -1;
3159 }
3160 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));
3163 return -1;
3164 }
3165 return 0;
3166 }
3167
3168 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)
3171 {
3172 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);
3174 strbuf_release(&sb);
3175 return ret;
3176 }
@@ -3182,36 +3182,36 @@ int is_branch(const char *refname)
3182
3183 /*
3184 * 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.
3187 */
3188 static int write_ref_to_lockfile(struct ref_lock *lock,
3188 - const unsigned char *sha1)
3189 + const unsigned char *sha1, struct strbuf *err)
3190 {
3191 static char term = '\n';
3192 struct object *o;
3193
3194 o = parse_object(sha1);
3195 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));
3199 unlock_ref(lock);
3198 - errno = EINVAL;
3200 return -1;
3201 }
3202 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);
3206 unlock_ref(lock);
3205 - errno = EINVAL;
3207 return -1;
3208 }
3209 if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 ||
3210 write_in_full(lock->lk->fd, &term, 1) != 1 ||
3211 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);
3214 unlock_ref(lock);
3214 - errno = save_errno;
3215 return -1;
3216 }
3217 return 0;
@@ -3223,12 +3223,17 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
3223 * necessary, using the specified lockmsg (which can be NULL).
3224 */
3225 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)
3228 {
3229 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 ||
3231 (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);
3237 unlock_ref(lock);
3238 return -1;
3239 }
@@ -3251,14 +3256,21 @@ static int commit_ref_update(struct ref_lock *lock,
3256 head_ref = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
3257 head_sha1, &head_flag);
3258 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 + }
3267 }
3268 if (commit_ref(lock)) {
3269 error("Couldn't set %s", lock->ref_name);
3270 unlock_ref(lock);
3271 return -1;
3272 }
3273 +
3274 unlock_ref(lock);
3275 return 0;
3276 }
@@ -3271,6 +3283,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
3283 int fd, len, written;
3284 char *git_HEAD = git_pathdup("%s", ref_target);
3285 unsigned char old_sha1[20], new_sha1[20];
3286 + struct strbuf err = STRBUF_INIT;
3287
3288 if (logmsg && read_ref(ref_target, old_sha1))
3289 hashclr(old_sha1);
@@ -3319,8 +3332,11 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
3332 #ifndef NO_SYMLINK_HEAD
3333 done:
3334 #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 + }
3340
3341 free(git_HEAD);
3342 return 0;
@@ -3956,14 +3972,19 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3972 * value, so we don't need to write it.
3973 */
3974 } 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 +
3979 /*
3980 * The lock was freed upon failure of
3981 * write_ref_to_lockfile():
3982 */
3983 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);
3988 ret = TRANSACTION_GENERIC_ERROR;
3989 goto cleanup;
3990 } else {
@@ -3989,11 +4010,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
4010
4011 if (update->flags & REF_NEEDS_COMMIT) {
4012 if (commit_ref_update(update->lock,
3992 - update->new_sha1, update->msg)) {
4013 + update->new_sha1, update->msg, err)) {
4014 /* freed by commit_ref_update(): */
4015 update->lock = NULL;
3995 - strbuf_addf(err, "Cannot update the ref '%s'.",
3996 - update->refname);
4016 ret = TRANSACTION_GENERIC_ERROR;
4017 goto cleanup;
4018 } else {
refs.h
+2 -2
@@ -189,9 +189,9 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
189 #define REF_NODEREF 0x01
190
191 /*
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.
193 */
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);
195
196 /** Reads log for the value of ref during at_time. **/
197 extern int read_ref_at(const char *refname, unsigned int flags,