From 9a7a183bd216fc7f824460e1f6d27886938a6152 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Mar 2013 10:36:43 -0800 Subject: for_each_reflog_ent(): extract a helper to process a single entry Split the logic that takes a single line of reflog entry in a strbuf, parses the message, and calls the callback function out of the loop into a separate helper function. Signed-off-by: Junio C Hamano --- refs.c | 59 ++++++++++++++++++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 29 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index da74a2b29a..9f702a763d 100644 --- a/refs.c +++ b/refs.c @@ -2290,6 +2290,34 @@ int read_ref_at(const char *refname, unsigned long at_time, int cnt, return 1; } +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 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); +} + int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long ofs, void *cb_data) { const char *logfile; @@ -2314,35 +2342,8 @@ int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long } } - 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 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; - } + while (!ret && !strbuf_getwholeline(&sb, logfp, '\n')) + ret = show_one_reflog_ent(&sb, fn, cb_data); fclose(logfp); strbuf_release(&sb); return ret; -- cgit v1.2.3 From 7ae07c1bd7bdab0bf0b36190c94e761cc593a890 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Mar 2013 10:45:25 -0800 Subject: for_each_recent_reflog_ent(): simplify opening of a reflog file There is no reason to use a temporary variable logfile. Signed-off-by: Junio C Hamano --- refs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 9f702a763d..e302521431 100644 --- a/refs.c +++ b/refs.c @@ -2320,13 +2320,11 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long ofs, void *cb_data) { - const char *logfile; FILE *logfp; struct strbuf sb = STRBUF_INIT; int ret = 0; - logfile = git_path("logs/%s", refname); - logfp = fopen(logfile, "r"); + logfp = fopen(git_path("logs/%s", refname), "r"); if (!logfp) return -1; -- cgit v1.2.3 From 98f85ff4b65b565bae0592ded494d67045cbd3bf Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Mar 2013 13:27:37 -0800 Subject: reflog: add for_each_reflog_ent_reverse() API "git checkout -" is a short-hand for "git checkout @{-1}" and the "@{nth}" notation for a negative number is to find nth previous checkout in the reflog of the HEAD to determine the name of the branch the user was on. We would want to find the nth most recent reflog entry that matches "checkout: moving from X to Y" for this. Unfortunately, reflog is implemented as an append-only file, and the API to iterate over its entries, for_each_reflog_ent(), reads the file in order, giving the entries from the oldest to newer. For the purpose of finding nth most recent one, this API forces us to record the last n entries in a rotating buffer and give the result out only after we read everything. To optimize for a common case of finding the nth most recent one for a small value of n, we also have a side API for_each_recent_reflog_ent() that starts reading near the end of the file, but it still has to read the entries in the "wrong" order. The implementation of understanding @{-1} uses this interface. This all becomes unnecessary if we add an API to let us iterate over reflog entries in the reverse order, from the newest to older. Signed-off-by: Junio C Hamano --- refs.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 86 insertions(+), 16 deletions(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index e302521431..8e24060d80 100644 --- a/refs.c +++ b/refs.c @@ -2318,30 +2318,89 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data); } -int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long ofs, void *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) { - FILE *logfp; struct strbuf sb = STRBUF_INIT; - int ret = 0; + FILE *logfp; + long pos; + int ret = 0, at_tail = 1; 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 < 0) + 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 (!ret && !strbuf_getwholeline(&sb, logfp, '\n')) + } + if (!ret && sb.len) ret = show_one_reflog_ent(&sb, fn, cb_data); + fclose(logfp); strbuf_release(&sb); return ret; @@ -2349,9 +2408,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 -- cgit v1.2.3 From e4ca819abff48af8e4a10059c88b3b1533b9f994 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 23 Mar 2013 17:16:46 +0000 Subject: refs.c: fix fread error handling fread returns the number of items read, with no special error return. Commit 98f85ff (reflog: add for_each_reflog_ent_reverse() API - 2013-03-08) introduced a call to fread which checks for an error with "nread < 0" which is tautological since nread is unsigned. The correct check in this case (which tries to read a single item) is "nread != 1". Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'refs.c') diff --git a/refs.c b/refs.c index 8e24060d80..aa79c93045 100644 --- a/refs.c +++ b/refs.c @@ -2357,7 +2357,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void return error("cannot seek back reflog for %s: %s", refname, strerror(errno)); nread = fread(buf, cnt, 1, logfp); - if (nread < 0) + if (nread != 1) return error("cannot read %d bytes from reflog for %s: %s", cnt, refname, strerror(errno)); pos -= cnt; -- cgit v1.2.3