git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH/RFC v3 1/6] reflog: refactor parsing and checking

From
Thomas Rast <trast@student.ethz.ch>
Date
Jan 17, 2009, 03:30 UTC
Message-ID
<1232163011-20088-2-git-send-email-trast@student.ethz.ch>
In-Reply-To
<1232163011-20088-1-git-send-email-trast@student.ethz.ch>

read_ref_at() and for_each_reflog_ent() both had parsing and error checking routines. Refactor into a separate function that fully parses a single entry. Note that this switches for_each_reflog_ent() from silently ignoring errors to die().

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 refs.c |  197 ++++++++++++++++++++++++++++++++--------------------------------
 1 files changed, 98 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 33ced65..4571fac 100644
--- a/refs.c
+++ b/refs.c
@@ -1337,24 +1337,68 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 	return 0;
 }
 
-static char *ref_msg(const char *line, const char *endp)
+/*
+ * Check and parse a single reflog line.  Assumes that there is only
+ * one newline in the range buf[0]..buf[len-1] (but does check that it
+ * is at buf[len-1]).
+ */
+static void parse_reflog_line(const char *buf, int len,
+			      unsigned char *osha1, unsigned char *nsha1,
+			      char **email,
+			      unsigned long *timestamp, int *tz,
+			      char **message,
+			      const char *logname)
 {
-	const char *ep;
-	line += 82;
-	ep = memchr(line, '\n', endp - line);
-	if (!ep)
-		ep = endp;
-	return xmemdupz(line, ep - line);
+	static char *retbuf = NULL;
+	static int retbufsz = 0;
+	char *tzstr, *email_end;
+
+	if (len < 83 || buf[len-1] != '\n')
+		die("Log %s is corrupt (entry too short or unterminated).", logname);
+
+	if (get_sha1_hex(buf, osha1) || buf[40] != ' ')
+		die("Log %s is corrupt (malformed old sha1).", logname);
+
+	if (get_sha1_hex(buf + 41, nsha1) || buf[81] != ' ')
+		die("Log %s is corrupt (malformed new sha1).", logname);
+
+	ALLOC_GROW(retbuf, len-82+1, retbufsz);
+	memcpy(retbuf, buf+82, len-82);
+	retbuf[len-82] = '\0';
+
+	email_end = strchr(retbuf, '>');
+	if (!email_end || email_end[1] != ' ')
+		die("Log %s is corrupt (malformed email field).", logname);
+
+	*email = retbuf;
+	email_end[1] = '\0';
+
+	*timestamp = strtoul(email_end + 2, &tzstr, 10);
+	if (!(*timestamp) || !tzstr || tzstr[0] != ' ' ||
+	    (tzstr[1] != '+' && tzstr[1] != '-') ||
+	    !isdigit(tzstr[2]) || !isdigit(tzstr[3]) ||
+	    !isdigit(tzstr[4]) || !isdigit(tzstr[5]))
+		die("Log %s is corrupt (malformed timezone).", logname);
+	if (!(tzstr[6] == '\t' || tzstr[6] == '\n'))
+		die("Log %s is corrupt (bad message field separator).", logname);
+	*tz = strtoul(tzstr, NULL, 10);
+
+	if (tzstr[6] == '\t')
+		*message = tzstr+7;
+	else
+		*message = tzstr+6;
 }
 
 int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
-	const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
-	char *tz_c;
+	const char *logfile, *logdata, *logend, *rec, *start;
+	char *email, *message;
 	int logfd, tz, reccnt = 0;
 	struct stat st;
 	unsigned long date;
-	unsigned char logged_sha1[20];
+	unsigned char new_sha1[20];
+	unsigned char old_sha1[20];
+	unsigned char next_sha1[20];
 	void *log_mapped;
 	size_t mapsz;
 
@@ -1370,86 +1414,55 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
 	logdata = log_mapped;
 	close(logfd);
 
-	lastrec = NULL;
 	rec = logend = logdata + st.st_size;
+	if (logdata < rec && *(rec-1) == '\n')
+		rec--;
 	while (logdata < rec) {
-		reccnt++;
-		if (logdata < rec && *(rec-1) == '\n')
-			rec--;
-		lastgt = NULL;
-		while (logdata < rec && *(rec-1) != '\n') {
-			rec--;
-			if (*rec == '>')
-				lastgt = rec;
-		}
-		if (!lastgt)
-			die("Log %s is corrupt.", logfile);
-		date = strtoul(lastgt + 1, &tz_c, 10);
+		start = memrchr(logdata, '\n', rec-logdata);
+		if (start)
+			start++;
+		else
+			start = logdata;
+		parse_reflog_line(start, rec-start+1,
+				  old_sha1, new_sha1,
+				  &email, &date, &tz, &message,
+				  logfile);
+
+		if (cutoff_time)
+			*cutoff_time = date;
+		if (cutoff_tz)
+			*cutoff_tz = tz;
+		if (cutoff_cnt)
+			*cutoff_cnt = reccnt;
+		if (msg)
+			*msg = message;
+
 		if (date <= at_time || cnt == 0) {
-			tz = strtoul(tz_c, NULL, 10);
-			if (msg)
-				*msg = ref_msg(rec, logend);
-			if (cutoff_time)
-				*cutoff_time = date;
-			if (cutoff_tz)
-				*cutoff_tz = tz;
-			if (cutoff_cnt)
-				*cutoff_cnt = reccnt - 1;
-			if (lastrec) {
-				if (get_sha1_hex(lastrec, logged_sha1))
-					die("Log %s is corrupt.", logfile);
-				if (get_sha1_hex(rec + 41, sha1))
-					die("Log %s is corrupt.", logfile);
-				if (hashcmp(logged_sha1, sha1)) {
-					fprintf(stderr,
-						"warning: Log %s has gap after %s.\n",
-						logfile, show_date(date, tz, DATE_RFC2822));
-				}
-			}
-			else if (date == at_time) {
-				if (get_sha1_hex(rec + 41, sha1))
-					die("Log %s is corrupt.", logfile);
-			}
-			else {
-				if (get_sha1_hex(rec + 41, logged_sha1))
-					die("Log %s is corrupt.", logfile);
-				if (hashcmp(logged_sha1, sha1)) {
-					fprintf(stderr,
-						"warning: Log %s unexpectedly ended on %s.\n",
-						logfile, show_date(date, tz, DATE_RFC2822));
-				}
-			}
+			if (reccnt && hashcmp(new_sha1, next_sha1))
+				fprintf(stderr,
+					"warning: Log %s has gap after %s.\n",
+					logfile, show_date(date, tz, DATE_RFC2822));
+			if (!reccnt && date < at_time && hashcmp(new_sha1, next_sha1))
+				fprintf(stderr,
+					"warning: Log %s unexpectedly ended on %s.\n",
+					logfile, show_date(date, tz, DATE_RFC2822));
+				/* leave caller's sha1 untouched */
+			else
+				hashcpy(sha1, new_sha1);
 			munmap(log_mapped, mapsz);
 			return 0;
 		}
-		lastrec = rec;
+
+		hashcpy(next_sha1, old_sha1);
+		rec = start-1;
 		if (cnt > 0)
 			cnt--;
+		reccnt++;
 	}
 
-	rec = logdata;
-	while (rec < logend && *rec != '>' && *rec != '\n')
-		rec++;
-	if (rec == logend || *rec == '\n')
-		die("Log %s is corrupt.", logfile);
-	date = strtoul(rec + 1, &tz_c, 10);
-	tz = strtoul(tz_c, NULL, 10);
-	if (get_sha1_hex(logdata, sha1))
-		die("Log %s is corrupt.", logfile);
-	if (is_null_sha1(sha1)) {
-		if (get_sha1_hex(logdata + 41, sha1))
-			die("Log %s is corrupt.", logfile);
-	}
-	if (msg)
-		*msg = ref_msg(logdata, logend);
-	munmap(log_mapped, mapsz);
+	hashcpy(sha1, new_sha1);
 
-	if (cutoff_time)
-		*cutoff_time = date;
-	if (cutoff_tz)
-		*cutoff_tz = tz;
-	if (cutoff_cnt)
-		*cutoff_cnt = reccnt;
+	munmap(log_mapped, mapsz);
 	return 1;
 }
 
@@ -1466,30 +1479,16 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 		return -1;
 	while (fgets(buf, sizeof(buf), logfp)) {
 		unsigned char osha1[20], nsha1[20];
-		char *email_end, *message;
+		char *email, *message;
 		unsigned long timestamp;
 		int len, tz;
 
-		/* old SP new SP name <email> SP time TAB msg LF */
 		len = strlen(buf);
-		if (len < 83 || buf[len-1] != '\n' ||
-		    get_sha1_hex(buf, osha1) || buf[40] != ' ' ||
-		    get_sha1_hex(buf + 41, nsha1) || buf[81] != ' ' ||
-		    !(email_end = strchr(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, buf+82, timestamp, tz, message, cb_data);
+		parse_reflog_line(buf, len,
+				  osha1, nsha1,
+				  &email, &timestamp, &tz, &message,
+				  logfile);
+		ret = fn(osha1, nsha1, email, timestamp, tz, message, cb_data);
 		if (ret)
 			break;
 	}

-- 
1.6.1.315.g92577
Previous: Thomas RastNext: Thomas Rast
Message 2 of 56 in “N-th last checked out branch”
  1. 0/6 N-th last checked out branchThomas Rast, Jan 17, 2009
  2. 1/6 reflog: refactor parsing and checkingThomas Rast, Jan 17, 2009
  3. 2/6 reflog: refactor log open+mmapThomas Rast, Jan 17, 2009
  4. 3/6 reflog: make for_each_reflog_ent use mmapThomas Rast, Jan 17, 2009
  5. 4/6 reflog: add backwards iteratorThomas Rast, Jan 17, 2009
  6. 5/6 sha1_name: implement @{-N} syntax for N-th last checked outThomas Rast, Jan 17, 2009
  7. 6/6 checkout: implement '@{-N}' and '-' special abbreviationsThomas Rast, Jan 17, 2009
  8. Johannes SchindelinJan 17, 2009
  9. Johannes SchindelinJan 17, 2009
  10. Johannes SchindelinJan 17, 2009
  11. Thomas RastJan 17, 2009
  12. 1/2 sha1_name: implement @{-N} syntax for N-th last checked outThomas Rast, Jan 17, 2009
  13. 2/2 checkout: implement '@{-N}' and '-' special abbreviationsThomas Rast, Jan 17, 2009
  14. interpret_nth_last_branch(): avoid traversing the reflogs twiceJohannes Schindelin, Jan 17, 2009
  15. 0/5 N-th last checked out branchThomas Rast, Jan 17, 2009
  16. 1/5 checkout: implement "@{-N}" shortcut name for N-th last branchThomas Rast, Jan 17, 2009
  17. 2/5 sha1_name: tweak @{-N} lookupThomas Rast, Jan 17, 2009
  18. 3/5 sha1_name: support @{-N} syntax in get_sha1()Thomas Rast, Jan 17, 2009
  19. 4/5 checkout: implement "-" abbreviation, add docs and testsThomas Rast, Jan 17, 2009
  20. 5/5 interpret_nth_last_branch(): avoid traversing the reflogs twiceThomas Rast, Jan 17, 2009
  21. 6/5 Fix parsing of @{-1}@{1}Johannes Schindelin, Jan 17, 2009
  22. Junio C HamanoJan 17, 2009
  23. Johannes SchindelinJan 17, 2009
  24. Junio C HamanoJan 17, 2009
  25. Johannes SchindelinJan 17, 2009
  26. Junio C HamanoJan 17, 2009
  27. Junio C HamanoJan 18, 2009
  28. Johannes SchindelinJan 17, 2009
  29. Junio C HamanoJan 17, 2009
  30. Johannes SchindelinJan 17, 2009
  31. Junio C HamanoJan 18, 2009
  32. Johannes SchindelinJan 18, 2009
  33. Junio C HamanoJan 18, 2009
  34. Johannes SchindelinJan 18, 2009
  35. Junio C HamanoJan 19, 2009
  36. Junio C HamanoJan 19, 2009
  37. Johannes SchindelinJan 19, 2009
  38. Thomas RastJan 20, 2009
  39. Johannes SchindelinJan 20, 2009
  40. Thomas RastJan 20, 2009
  41. interpret_nth_last_branch(): plug small memleakJunio C Hamano, Jan 20, 2009
  42. Johannes SchindelinJan 20, 2009
  43. Introduce for_each_recent_reflog_ent().Junio C Hamano, Jan 20, 2009
  44. Johannes SchindelinJan 20, 2009
  45. Junio C HamanoJan 20, 2009
  46. Fix invalid memory accessJohannes Schindelin, Jan 21, 2009
  47. Junio C HamanoJan 21, 2009
  48. Thomas RastJan 21, 2009
  49. Junio C HamanoJan 21, 2009
  50. Johannes SchindelinJan 21, 2009
  51. Thomas RastJan 24, 2009
  52. t1505: remove debugging cruftThomas Rast, Jan 24, 2009
  53. Junio C HamanoJan 25, 2009
  54. Johannes SchindelinJan 21, 2009
  55. @{-<n>}: avoid crash with corrupt reflogJohannes Schindelin, Jan 19, 2009
  56. Johannes SchindelinJan 19, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.