git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Ronnie Sahlberg <sahlberg@google.com>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v2 06/11] refs.c: move ref parsing code out of resolve_ref()
Date: Wed, 15 Oct 2014 17:06:18 +0200	[thread overview]
Message-ID: <1413385583-4872-7-git-send-email-mhagger@alum.mit.edu> (raw)
In-Reply-To: <1413385583-4872-1-git-send-email-mhagger@alum.mit.edu>

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

This function will soon have a second caller.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

I leave parse_ref() a public function for the reason explained by
Duy [1].

[1] http://article.gmane.org/gmane.comp.version-control.git/254274

 cache.h |  11 ++++
 refs.c  | 200 ++++++++++++++++++++++++++++++++++------------------------------
 2 files changed, 118 insertions(+), 93 deletions(-)

diff --git a/cache.h b/cache.h
index e36084d..d2b8399 100644
--- a/cache.h
+++ b/cache.h
@@ -1005,6 +1005,17 @@ extern int read_ref(const char *refname, unsigned char *sha1);
 extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1, int reading, int *flag);
 extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);
 extern int resolve_ref(const char *refname, struct strbuf *result, unsigned char *sha1, int reading, int *flag);
+/*
+ * Given a ref in "ref" and its path, returns
+ *
+ * -2  failed to open with ENOENT, the caller is responsible for
+ *     checking missing loose ref (see resolve_ref for example)
+ * -1  if there's an error, "refname" can no longer be trusted, "flag" may
+ *     be set. errno is valid.
+ *  0  this is a symref, "refname" now contains the linked ref
+ * +1  normal ref, "sha1" is valid
+ */
+extern int parse_ref(const char *path, struct strbuf *refname, unsigned char *sha1, int *flag);
 
 extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
 extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/refs.c b/refs.c
index 29ea7e0..3acd83e 100644
--- a/refs.c
+++ b/refs.c
@@ -1397,6 +1397,104 @@ static int handle_missing_loose_ref(const char *refname,
 	}
 }
 
+int parse_ref(const char *path, struct strbuf *refname,
+	      unsigned char *sha1, int *flag)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	struct stat st;
+	const char *buf;
+	int ret;
+
+	/*
+	 * We might have to loop back here to avoid a race condition:
+	 * first we lstat() the file, then we try to read it as a link
+	 * or as a file.  But if somebody changes the type of the file
+	 * (file <-> directory <-> symlink) between the lstat() and
+	 * reading, then we don't want to report that as an error but
+	 * rather try again starting with the lstat().
+	 */
+stat_ref:
+	if (lstat(path, &st) < 0)
+		return errno == ENOENT ? -2 : -1;
+
+	/* Follow "normalized" - ie "refs/.." symlinks by hand */
+	if (S_ISLNK(st.st_mode)) {
+		struct strbuf new_path = STRBUF_INIT;
+		if (strbuf_readlink(&new_path, path, 256) < 0) {
+			strbuf_release(&new_path);
+			if (errno == ENOENT || errno == EINVAL)
+				/* inconsistent with lstat; retry */
+				goto stat_ref;
+			else
+				return -1;
+		}
+		if (starts_with(new_path.buf, "refs/") &&
+		    !check_refname_format(new_path.buf, 0)) {
+			strbuf_reset(refname);
+			strbuf_addbuf(refname, &new_path);
+			if (flag)
+				*flag |= REF_ISSYMREF;
+			strbuf_release(&new_path);
+			return 0;
+		}
+		strbuf_release(&new_path);
+	}
+
+	/* Is it a directory? */
+	if (S_ISDIR(st.st_mode)) {
+		errno = EISDIR;
+		return -1;
+	}
+
+	/*
+	 * Anything else, just open it and try to use it as
+	 * a ref
+	 */
+	if (strbuf_read_file(&buffer, path, 256) < 0) {
+		strbuf_release(&buffer);
+		if (errno == ENOENT)
+			/* inconsistent with lstat; retry */
+			goto stat_ref;
+		else
+			return -1;
+	}
+	strbuf_rtrim(&buffer);
+
+	if (skip_prefix(buffer.buf, "ref:", &buf)) {
+		/* It is a symbolic ref */
+		if (flag)
+			*flag |= REF_ISSYMREF;
+		while (isspace(*buf))
+			buf++;
+		if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
+			if (flag)
+				*flag |= REF_ISBROKEN;
+			strbuf_release(&buffer);
+			errno = EINVAL;
+			return -1;
+		}
+		strbuf_reset(refname);
+		strbuf_add(refname, buf, buffer.buf + buffer.len - buf);
+		strbuf_release(&buffer);
+		return 0;
+	}
+
+	/*
+	 * Please note that FETCH_HEAD has a second line
+	 * containing other data.
+	 */
+	if (get_sha1_hex(buffer.buf, sha1) ||
+	    (buffer.buf[40] != '\0' && !isspace(buffer.buf[40]))) {
+		if (flag)
+			*flag |= REF_ISBROKEN;
+		errno = EINVAL;
+		ret = -1;
+	} else
+		ret = 1;
+	strbuf_release(&buffer);
+	return ret;
+}
+
 /*
  * 'result' content will be destroyed. Its value may be undefined if
  * resolve_ref returns -1.
@@ -1406,9 +1504,8 @@ static int handle_missing_loose_ref(const char *refname,
 int resolve_ref(const char *refname, struct strbuf *result,
 		unsigned char *sha1, int reading, int *flag)
 {
-	struct strbuf buffer = STRBUF_INIT;
 	int depth = MAXDEPTH;
-	int ret = -1;
+	int ret = 0;
 
 	if (flag)
 		*flag = 0;
@@ -1421,107 +1518,24 @@ int resolve_ref(const char *refname, struct strbuf *result,
 	strbuf_reset(result);
 	strbuf_addstr(result, refname);
 
-	for (;;) {
+	while (!ret) {
 		char path[PATH_MAX];
-		struct stat st;
-		const char *buf;
 
 		if (--depth < 0) {
 			errno = ELOOP;
+			ret = -1;
 			break;
 		}
 
 		git_snpath(path, sizeof(path), "%s", result->buf);
-
-		/*
-		 * We might have to loop back here to avoid a race
-		 * condition: first we lstat() the file, then we try
-		 * to read it as a link or as a file.  But if somebody
-		 * changes the type of the file (file <-> directory
-		 * <-> symlink) between the lstat() and reading, then
-		 * we don't want to report that as an error but rather
-		 * try again starting with the lstat().
-		 */
-	stat_ref:
-		if (lstat(path, &st) < 0) {
-			if (errno == ENOENT)
-				ret = handle_missing_loose_ref(result->buf, sha1,
-							       reading, flag);
-			break;
+		ret = parse_ref(path, result, sha1, flag);
+		if (ret == -2) {
+			ret = handle_missing_loose_ref(result->buf, sha1,
+						       reading, flag);
+			ret = ret ? -1 : 1;
 		}
-
-		/* Follow "normalized" - ie "refs/.." symlinks by hand */
-		if (S_ISLNK(st.st_mode)) {
-			/* no need to reset buffer, strbuf_readlink does that */
-			if (strbuf_readlink(&buffer, path, 256) < 0) {
-				if (errno == ENOENT || errno == EINVAL)
-					/* inconsistent with lstat; retry */
-					goto stat_ref;
-				else
-					break;
-			}
-			if (starts_with(buffer.buf, "refs/") &&
-			    !check_refname_format(buffer.buf, 0)) {
-				strbuf_reset(result);
-				strbuf_addbuf(result, &buffer);
-				if (flag)
-					*flag |= REF_ISSYMREF;
-				continue;
-			}
-		}
-
-		/* Is it a directory? */
-		if (S_ISDIR(st.st_mode)) {
-			errno = EISDIR;
-			break;
-		}
-
-		/*
-		 * Anything else, just open it and try to use it as
-		 * a ref
-		 */
-		strbuf_reset(&buffer);
-		if (strbuf_read_file(&buffer, path, 256) < 0) {
-			if (errno == ENOENT)
-				/* inconsistent with lstat; retry */
-				goto stat_ref;
-			else
-				break;
-		}
-		strbuf_rtrim(&buffer);
-
-		if (skip_prefix(buffer.buf, "ref:", &buf)) {
-			/* It is a symbolic ref */
-			if (flag)
-				*flag |= REF_ISSYMREF;
-			while (isspace(*buf))
-				buf++;
-			if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
-				if (flag)
-					*flag |= REF_ISBROKEN;
-				errno = EINVAL;
-				break;
-			}
-			strbuf_reset(result);
-			strbuf_add(result, buf, buffer.buf + buffer.len - buf);
-			continue;
-		}
-
-		/*
-		 * It must be a normal ref. Please note that
-		 * FETCH_HEAD has a second line containing other data.
-		 */
-		if (get_sha1_hex(buffer.buf, sha1) ||
-		    (buffer.buf[40] != '\0' && !isspace(buffer.buf[40]))) {
-			if (flag)
-				*flag |= REF_ISBROKEN;
-			errno = EINVAL;
-		} else
-			ret = 0;
-		break;
 	}
-	strbuf_release(&buffer);
-	return ret;
+	return ret > 0 ? 0 : -1;
 }
 
 const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int reading, int *flag)
-- 
2.1.1

  parent reply	other threads:[~2014-10-15 15:07 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-15 15:06 [PATCH v2 00/11] Consolidate ref parsing code Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 01/11] strbuf_read_file(): preserve errno on failure Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 02/11] handle_missing_loose_ref(): return an int Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 03/11] resolve_ref_unsafe(): reverse the logic of the symref conditional Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 04/11] resolve_ref_unsafe(): use skip_prefix() to skip over "ref:" Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 05/11] refs.c: refactor resolve_ref_unsafe() to use strbuf internally Michael Haggerty
2014-10-15 15:06 ` Michael Haggerty [this message]
2014-10-15 15:06 ` [PATCH v2 07/11] handle_missing_loose_ref(): inline function Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 08/11] resolve_gitlink_ref_recursive(): drop arbitrary refname length limit Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 09/11] refs.c: rewrite resolve_gitlink_ref() to use parse_ref() Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 10/11] resove_gitlink_packed_ref(): inline function Michael Haggerty
2014-10-15 15:06 ` [PATCH v2 11/11] resolve_gitlink_ref(): remove redundant test Michael Haggerty
2014-10-16 20:47 ` [PATCH v2 00/11] Consolidate ref parsing code Junio C Hamano
2014-10-16 21:51   ` Junio C Hamano
2014-10-16 23:23   ` Michael Haggerty
2014-10-16 23:53 ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1413385583-4872-7-git-send-email-mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sahlberg@google.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).