Subject: Bug: bad errno report from lock_ref_sha1_basic() when contents bad

From: Thomas Rast <[email-masked]>
Date: Mon, 2 Apr 2012 12:04:29 +0200

Hi Junio,

User "mbrochh" on IRC reported a result along the lines of

  error: unable to resolve reference refs/remotes/origin/master: Success
  ! [new branch] master -> origin/master (unable to update local ref)

from 'git fetch' (or pull actually, but clearly it's the fetch part
failing).  This error message is from lock_ref_sha1_basic(), and
suggests a confused error exit taken with errno=0.

I can reproduce the problem by writing a bit of garbage to a remote ref
and then fetching, though I usually get some other error in the message
(usually ENOENT, which is equally wrong).

As far as I can tell it's caused by resolve_ref_unsafe() returning NULL
when the content checks fail:

	if (get_sha1_hex(buffer, sha1) || (buffer[40] != '\0' && !isspace(buffer[40]))) {
		if (flag)
			*flag |= REF_ISBROKEN;
		return NULL;
	}

This used to warn before your 5595635 (resolve_ref(): report breakage to
the caller without warning, 2011-10-19), but otherwise suffered from the
same problem.

As far as I can see lock_ref_sha1_basic() needs a similar check for
REF_ISBROKEN (and presumably the only fix is to nuke it so maybe it
should even suggest that to the user).  But in general, the "errno may
be crucial or worthless" interface of lock_ref_sha1_basic() seems a bit
broken...

-- 
Thomas Rast
trast@{inf,student}.ethz.ch