Making errno from write_ref_sha1() meaningful, which should fix

* a bug in "git checkout -b" where it prints strerror(errno)
  despite errno possibly being zero or clobbered

* a bug in "git fetch"'s s_update_ref, which trusts the result of an
  errno == ENOTDIR check to detect D/F conflicts

Signed-off-by: Ronnie Sahlberg <sahlb...@google.com>
---
 refs.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index a4c728c..31ae63e 100644
--- a/refs.c
+++ b/refs.c
@@ -1951,6 +1951,7 @@ static int remove_empty_directories(const char *file)
        result = remove_dir_recursively(&path, REMOVE_DIR_EMPTY_ONLY);
        save_errno = errno;
 
+       errno = save_errno;
        strbuf_release(&path);
        errno = save_errno;
 
@@ -2831,8 +2832,19 @@ static int log_ref_write(const char *refname, const 
unsigned char *old_sha1,
                len += copy_msg(logrec + len - 1, msg) - 1;
        written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;
        free(logrec);
-       if (close(logfd) != 0 || written != len)
-               return error("Unable to append to %s", log_file);
+       if (written != len) {
+               int save_errno = 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;
+               return -1;
+       }
        return 0;
 }
 
@@ -2841,14 +2853,17 @@ static int is_branch(const char *refname)
        return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
 }
 
+/* This function must return a meaningful errno */
 int write_ref_sha1(struct ref_lock *lock,
        const unsigned char *sha1, const char *logmsg)
 {
        static char term = '\n';
        struct object *o;
 
-       if (!lock)
+       if (!lock) {
+               errno = EINVAL;
                return -1;
+       }
        if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
                unlock_ref(lock);
                return 0;
@@ -2858,19 +2873,23 @@ int write_ref_sha1(struct ref_lock *lock,
                error("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);
                unlock_ref(lock);
+               errno = EINVAL;
                return -1;
        }
        if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
-           write_in_full(lock->lock_fd, &term, 1) != 1
-               || close_ref(lock) < 0) {
+           write_in_full(lock->lock_fd, &term, 1) != 1 ||
+           close_ref(lock) < 0) {
+               int save_errno = errno;
                error("Couldn't write %s", lock->lk->filename);
                unlock_ref(lock);
+               errno = save_errno;
                return -1;
        }
        clear_loose_ref_cache(&ref_cache);
-- 
2.0.0.599.g83ced0e

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to