refs.c: log_ref_write should try to return meaningful errno

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 <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>

Ronnie Sahlberg committed Jun 20, 2014 at 07:42 UTC dc615de8610c807633c63313937056bf45df0b8f
1 file changed +23 -5
refs.c
+23 -5
@@ -2859,8 +2859,19 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
2859 len += copy_msg(logrec + len - 1, msg) - 1;
2860 written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;
2861 free(logrec);
2862 - if (close(logfd) != 0 || written != len)
2863 - return error("Unable to append to %s", log_file);
2862 + if (written != len) {
2863 + int save_errno = errno;
2864 + close(logfd);
2865 + error("Unable to append to %s", log_file);
2866 + errno = save_errno;
2867 + return -1;
2868 + }
2869 + if (close(logfd)) {
2870 + int save_errno = errno;
2871 + error("Unable to append to %s", log_file);
2872 + errno = save_errno;
2873 + return -1;
2874 + }
2875 return 0;
2876 }
2877
@@ -2869,14 +2880,17 @@ static int is_branch(const char *refname)
2880 return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
2881 }
2882
2883 +/* This function must return a meaningful errno */
2884 int write_ref_sha1(struct ref_lock *lock,
2885 const unsigned char *sha1, const char *logmsg)
2886 {
2887 static char term = '\n';
2888 struct object *o;
2889
2878 - if (!lock)
2890 + if (!lock) {
2891 + errno = EINVAL;
2892 return -1;
2893 + }
2894 if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
2895 unlock_ref(lock);
2896 return 0;
@@ -2886,19 +2900,23 @@ int write_ref_sha1(struct ref_lock *lock,
2900 error("Trying to write ref %s with nonexistent object %s",
2901 lock->ref_name, sha1_to_hex(sha1));
2902 unlock_ref(lock);
2903 + errno = EINVAL;
2904 return -1;
2905 }
2906 if (o->type != OBJ_COMMIT && is_branch(lock->ref_name)) {
2907 error("Trying to write non-commit object %s to branch %s",
2908 sha1_to_hex(sha1), lock->ref_name);
2909 unlock_ref(lock);
2910 + errno = EINVAL;
2911 return -1;
2912 }
2913 if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
2898 - write_in_full(lock->lock_fd, &term, 1) != 1
2899 - || close_ref(lock) < 0) {
2914 + write_in_full(lock->lock_fd, &term, 1) != 1 ||
2915 + close_ref(lock) < 0) {
2916 + int save_errno = errno;
2917 error("Couldn't write %s", lock->lk->filename);
2918 unlock_ref(lock);
2919 + errno = save_errno;
2920 return -1;
2921 }
2922 clear_loose_ref_cache(&ref_cache);