Merge branch 'jc/reflog-reverse-walk'

An internal function used to implement "git checkout @{-1}" was
hard to use correctly.

* jc/reflog-reverse-walk:
  refs.c: fix fread error handling
  reflog: add for_each_reflog_ent_reverse() API
  for_each_recent_reflog_ent(): simplify opening of a reflog file
  for_each_reflog_ent(): extract a helper to process a single entry

3 files changed, 134 insertions, 77 deletions

diff --git a/refs.c b/refs.c
index e2b760d0ba..de2d8eb866 100644
--- a/refs.c
+++ b/refs.c
@@ -2332,59 +2332,117 @@ int read_ref_at(const char *refname, unsigned long at_time, int cnt,
 	return 1;
 }
 
-int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long ofs, void *cb_data)
+static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
+{
+	unsigned char osha1[20], nsha1[20];
+	char *email_end, *message;
+	unsigned long timestamp;
+	int tz;
+
+	/* old SP new SP name <email> SP time TAB msg LF */
+	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
+	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
+	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
+	    !(email_end = strchr(sb->buf + 82, '>')) ||
+	    email_end[1] != ' ' ||
+	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
+	    !message || message[0] != ' ' ||
+	    (message[1] != '+' && message[1] != '-') ||
+	    !isdigit(message[2]) || !isdigit(message[3]) ||
+	    !isdigit(message[4]) || !isdigit(message[5]))
+		return 0; /* corrupt? */
+	email_end[1] = '\0';
+	tz = strtol(message + 1, NULL, 10);
+	if (message[6] != '\t')
+		message += 6;
+	else
+		message += 7;
+	return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data);
+}
+
+static char *find_beginning_of_line(char *bob, char *scan)
+{
+	while (bob < scan && *(--scan) != '\n')
+		; /* keep scanning backwards */
+	/*
+	 * Return either beginning of the buffer, or LF at the end of
+	 * the previous line.
+	 */
+	return scan;
+}
+
+int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data)
 {
-	const char *logfile;
-	FILE *logfp;
 	struct strbuf sb = STRBUF_INIT;
-	int ret = 0;
+	FILE *logfp;
+	long pos;
+	int ret = 0, at_tail = 1;
 
-	logfile = git_path("logs/%s", refname);
-	logfp = fopen(logfile, "r");
+	logfp = fopen(git_path("logs/%s", refname), "r");
 	if (!logfp)
 		return -1;
 
-	if (ofs) {
-		struct stat statbuf;
-		if (fstat(fileno(logfp), &statbuf) ||
-		    statbuf.st_size < ofs ||
-		    fseek(logfp, -ofs, SEEK_END) ||
-		    strbuf_getwholeline(&sb, logfp, '\n')) {
-			fclose(logfp);
-			strbuf_release(&sb);
-			return -1;
+	/* Jump to the end */
+	if (fseek(logfp, 0, SEEK_END) < 0)
+		return error("cannot seek back reflog for %s: %s",
+			     refname, strerror(errno));
+	pos = ftell(logfp);
+	while (!ret && 0 < pos) {
+		int cnt;
+		size_t nread;
+		char buf[BUFSIZ];
+		char *endp, *scanp;
+
+		/* Fill next block from the end */
+		cnt = (sizeof(buf) < pos) ? sizeof(buf) : pos;
+		if (fseek(logfp, pos - cnt, SEEK_SET))
+			return error("cannot seek back reflog for %s: %s",
+				     refname, strerror(errno));
+		nread = fread(buf, cnt, 1, logfp);
+		if (nread != 1)
+			return error("cannot read %d bytes from reflog for %s: %s",
+				     cnt, refname, strerror(errno));
+		pos -= cnt;
+
+		scanp = endp = buf + cnt;
+		if (at_tail && scanp[-1] == '\n')
+			/* Looking at the final LF at the end of the file */
+			scanp--;
+		at_tail = 0;
+
+		while (buf < scanp) {
+			/*
+			 * terminating LF of the previous line, or the beginning
+			 * of the buffer.
+			 */
+			char *bp;
+
+			bp = find_beginning_of_line(buf, scanp);
+
+			if (*bp != '\n') {
+				strbuf_splice(&sb, 0, 0, buf, endp - buf);
+				if (pos)
+					break; /* need to fill another block */
+				scanp = buf - 1; /* leave loop */
+			} else {
+				/*
+				 * (bp + 1) thru endp is the beginning of the
+				 * current line we have in sb
+				 */
+				strbuf_splice(&sb, 0, 0, bp + 1, endp - (bp + 1));
+				scanp = bp;
+				endp = bp + 1;
+			}
+			ret = show_one_reflog_ent(&sb, fn, cb_data);
+			strbuf_reset(&sb);
+			if (ret)
+				break;
 		}
-	}
 
-	while (!strbuf_getwholeline(&sb, logfp, '\n')) {
-		unsigned char osha1[20], nsha1[20];
-		char *email_end, *message;
-		unsigned long timestamp;
-		int tz;
-
-		/* old SP new SP name <email> SP time TAB msg LF */
-		if (sb.len < 83 || sb.buf[sb.len - 1] != '\n' ||
-		    get_sha1_hex(sb.buf, osha1) || sb.buf[40] != ' ' ||
-		    get_sha1_hex(sb.buf + 41, nsha1) || sb.buf[81] != ' ' ||
-		    !(email_end = strchr(sb.buf + 82, '>')) ||
-		    email_end[1] != ' ' ||
-		    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
-		    !message || message[0] != ' ' ||
-		    (message[1] != '+' && message[1] != '-') ||
-		    !isdigit(message[2]) || !isdigit(message[3]) ||
-		    !isdigit(message[4]) || !isdigit(message[5]))
-			continue; /* corrupt? */
-		email_end[1] = '\0';
-		tz = strtol(message + 1, NULL, 10);
-		if (message[6] != '\t')
-			message += 6;
-		else
-			message += 7;
-		ret = fn(osha1, nsha1, sb.buf + 82, timestamp, tz, message,
-			 cb_data);
-		if (ret)
-			break;
 	}
+	if (!ret && sb.len)
+		ret = show_one_reflog_ent(&sb, fn, cb_data);
+
 	fclose(logfp);
 	strbuf_release(&sb);
 	return ret;
@@ -2392,9 +2450,20 @@ int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long
 
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_data)
 {
-	return for_each_recent_reflog_ent(refname, fn, 0, cb_data);
-}
+	FILE *logfp;
+	struct strbuf sb = STRBUF_INIT;
+	int ret = 0;
+
+	logfp = fopen(git_path("logs/%s", refname), "r");
+	if (!logfp)
+		return -1;
 
+	while (!ret && !strbuf_getwholeline(&sb, logfp, '\n'))
+		ret = show_one_reflog_ent(&sb, fn, cb_data);
+	fclose(logfp);
+	strbuf_release(&sb);
+	return ret;
+}
 /*
  * Call fn for each reflog in the namespace indicated by name.  name
  * must be empty or end with '/'.  Name will be used as a scratch
diff --git a/refs.h b/refs.h
index 1b2e2d3a98..a35eafc4ee 100644
--- a/refs.h
+++ b/refs.h
@@ -103,7 +103,7 @@ extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,
 /* iterate over reflog entries */
 typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_data);
-int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long, void *cb_data);
+int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data);
 
 /*
  * Calls the specified function for each reflog file until it returns nonzero,
diff --git a/sha1_name.c b/sha1_name.c
index c50630a3ea..2fbda48e02 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -856,8 +856,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,
 }
 
 struct grab_nth_branch_switch_cbdata {
-	long cnt, alloc;
-	struct strbuf *buf;
+	int remaining;
+	struct strbuf buf;
 };
 
 static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
@@ -867,7 +867,6 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	struct grab_nth_branch_switch_cbdata *cb = cb_data;
 	const char *match = NULL, *target = NULL;
 	size_t len;
-	int nth;
 
 	if (!prefixcmp(message, "checkout: moving from ")) {
 		match = message + strlen("checkout: moving from ");
@@ -876,11 +875,12 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 
 	if (!match || !target)
 		return 0;
-
-	len = target - match;
-	nth = cb->cnt++ % cb->alloc;
-	strbuf_reset(&cb->buf[nth]);
-	strbuf_add(&cb->buf[nth], match, len);
+	if (--(cb->remaining) == 0) {
+		len = target - match;
+		strbuf_reset(&cb->buf);
+		strbuf_add(&cb->buf, match, len);
+		return 1; /* we are done */
+	}
 	return 0;
 }
 
@@ -891,7 +891,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
 {
 	long nth;
-	int i, retval;
+	int retval;
 	struct grab_nth_branch_switch_cbdata cb;
 	const char *brace;
 	char *num_end;
@@ -901,34 +901,22 @@ static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
 	brace = strchr(name, '}');
 	if (!brace)
 		return -1;
-	nth = strtol(name+3, &num_end, 10);
+	nth = strtol(name + 3, &num_end, 10);
 	if (num_end != brace)
 		return -1;
 	if (nth <= 0)
 		return -1;
-	cb.alloc = nth;
-	cb.buf = xmalloc(nth * sizeof(struct strbuf));
-	for (i = 0; i < nth; i++)
-		strbuf_init(&cb.buf[i], 20);
-	cb.cnt = 0;
+	cb.remaining = nth;
+	strbuf_init(&cb.buf, 20);
+
 	retval = 0;
-	for_each_recent_reflog_ent("HEAD", grab_nth_branch_switch, 40960, &cb);
-	if (cb.cnt < nth) {
-		cb.cnt = 0;
-		for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+	if (0 < for_each_reflog_ent_reverse("HEAD", grab_nth_branch_switch, &cb)) {
+		strbuf_reset(buf);
+		strbuf_add(buf, cb.buf.buf, cb.buf.len);
+		retval = brace - name + 1;
 	}
-	if (cb.cnt < nth)
-		goto release_return;
-	i = cb.cnt % nth;
-	strbuf_reset(buf);
-	strbuf_add(buf, cb.buf[i].buf, cb.buf[i].len);
-	retval = brace-name+1;
-
-release_return:
-	for (i = 0; i < nth; i++)
-		strbuf_release(&cb.buf[i]);
-	free(cb.buf);
 
+	strbuf_release(&cb.buf);
 	return retval;
 }