Restyle the sources and fix the audit's findings

1 file changed, 274 insertions, 233 deletions

diff --git a/source/ui-log.c b/source/ui-log.c
index 5eb9a74..f80ed16 100644
--- a/source/ui-log.c
+++ b/source/ui-log.c
@@ -1,24 +1,32 @@
-/* ui-log.c: functions for log output
- *
- * Copyright (C) 2006-2014 cgit Development Team <cgit@lists.zx2c4.com>
- *
- * Licensed under GNU General Public License v2
- *   (see LICENSE.txt for full license text)
+/*
+ * The list of commits behind the log page and behind the recent commits on a
+ * repository's summary page. Each commit becomes a table row, with optional
+ * columns for the commit graph and for the files and lines it touched.
+ * Following a single path lives here as well, because deciding whether a
+ * commit belongs in a followed listing means diffing it against its parent
+ * first. The branch and tag labels beside a commit are drawn here too, since
+ * the commit page shows the same set.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
-#include "ui-log.h"
+#include "filter.h"
 #include "html.h"
+#include "parsing.h"
+#include "shared.h"
+#include "ui-log.h"
 #include "ui-shared.h"
-#include "strvec.h"
 
-static int files, add_lines, rem_lines, lines_counted;
+// A commit older than this is dated absolutely rather than relative to now.
+#define MAX_RELATIVE_AGE (SECONDS_PER_WEEK * 2)
 
-/*
- * The list of available column colors in the commit graph.
- */
+// Wrapping is skipped when max-message-len is under this, since the wrap
+// symbol takes four bytes of whatever it allows.
+#define MIN_WRAP_LEN 15
+
+// git wants the index of the closing entry rather than a count, and it has to
+// be handed these before the revision walk creates the graph.
 static const char *column_colors_html[] = {
 	"<span class='column1'>",
 	"<span class='column2'>",
@@ -31,130 +39,87 @@ static const char *column_colors_html[] = {
 
 #define COLUMN_COLORS_HTML_MAX (ARRAY_SIZE(column_colors_html) - 1)
 
-static void count_lines(char *line, int size)
+// counts_ready says diff has already run for this commit, which happens when a
+// path is being followed, so the row must not run it a second time.
+static int file_count, added_lines, removed_lines, counts_ready;
+
+static void count_line(char *line, int len)
 {
-	if (size <= 0)
+	if (len <= 0)
 		return;
 
 	if (line[0] == '+')
-		add_lines++;
-
+		added_lines++;
 	else if (line[0] == '-')
-		rem_lines++;
+		removed_lines++;
 }
 
-static void inspect_files(struct diff_filepair *pair)
+static void count_file(struct diff_filepair *pair)
 {
+	// cgit_diff_files needs somewhere to report the blob sizes and
+	// whether the pair is binary, none of which these columns show.
 	unsigned long old_size = 0;
 	unsigned long new_size = 0;
 	int binary = 0;
 
-	files++;
+	file_count++;
 	if (ctx.repo->enable_log_linecount)
 		cgit_diff_files(&pair->one->oid, &pair->two->oid, &old_size,
 				&new_size, &binary, 0, ctx.qry.ignorews,
-				count_lines);
-}
-
-void cgit_print_commit_decorations(struct commit *commit)
-{
-	const struct name_decoration *deco;
-	static char buf[1024];
-
-	buf[sizeof(buf) - 1] = 0;
-	deco = get_name_decoration(&commit->object);
-	if (!deco)
-		return;
-	html("<span class='decoration'>");
-	while (deco) {
-		struct object_id oid_tag, peeled;
-		int is_annotated = 0;
-
-		strlcpy(buf, prettify_refname(deco->name), sizeof(buf));
-		switch(deco->type) {
-		case DECORATION_NONE:
-			/* If the git-core doesn't recognize it,
-			 * don't display anything. */
-			break;
-		case DECORATION_REF_LOCAL:
-			cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
-				ctx.qry.vpath, 0, NULL, NULL,
-				ctx.qry.showmsg, 0);
-			break;
-		case DECORATION_REF_TAG:
-			if (!refs_read_ref(get_main_ref_store(the_repository), deco->name, &oid_tag) &&
-			    !peel_object(the_repository, &oid_tag, &peeled, PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE))
-				is_annotated = !oideq(&oid_tag, &peeled);
-			cgit_tag_link(buf, NULL, is_annotated ? "tag-annotated-deco" : "tag-deco", buf);
-			break;
-		case DECORATION_REF_REMOTE:
-			if (!ctx.repo->enable_remote_branches)
-				break;
-			cgit_log_link(buf, NULL, "remote-deco", NULL,
-				oid_to_hex(&commit->object.oid),
-				ctx.qry.vpath, 0, NULL, NULL,
-				ctx.qry.showmsg, 0);
-			break;
-		default:
-			cgit_commit_link(buf, NULL, "deco", ctx.qry.head,
-					oid_to_hex(&commit->object.oid),
-					ctx.qry.vpath);
-			break;
-		}
-		deco = deco->next;
-	}
-	html("</span>");
+				count_line);
 }
 
-static void handle_rename(struct diff_filepair *pair)
+/*
+ * Moves the query path to the name the file carries in the commit being
+ * examined, so the commit and diff links in its row lead to the path that
+ * commit knows.
+ */
+static void follow_path(struct diff_filepair *pair)
 {
-	/*
-	 * After we have seen a rename, we generate links to the previous
-	 * name of the file so that commit & diff views get fed the path
-	 * that is correct for the commit they are showing, avoiding the
-	 * need to walk the entire history leading back to every commit we
-	 * show in order detect renames.
-	 */
-	if (0 != strcmp(ctx.qry.vpath, pair->two->path)) {
-		free(ctx.qry.vpath);
+	if (strcmp(ctx.qry.vpath, pair->two->path) != 0) {
+		// The first vpath is the same allocation as ctx.qry.path, so
+		// freeing it here would leave that field pointing at memory
+		// this function had already released.
+		if (ctx.qry.vpath != ctx.qry.path)
+			free(ctx.qry.vpath);
 		ctx.qry.vpath = xstrdup(pair->two->path);
 	}
-	inspect_files(pair);
+	count_file(pair);
 }
 
-static int show_commit(struct commit *commit, struct rev_info *revs)
+/*
+ * Whether a commit belongs in the listing. When a single path is being
+ * followed the commit is diffed against its one parent here, and the counts
+ * that fall out of that diff are kept so the row does not repeat the work.
+ */
+static int should_show(struct commit *commit, struct rev_info *revs)
 {
 	struct commit_list *parents = commit->parents;
 	struct commit *parent;
 	int found = 0, saved_fmt;
 	struct diff_flags saved_flags = revs->diffopt.flags;
 
-	/* Always show if we're not in "follow" mode with a single file. */
 	if (!ctx.qry.follow)
 		return 1;
 
-	/*
-	 * In "follow" mode, we don't show merges.  This is consistent with
-	 * "git log --follow -- <file>".
-	 */
+	// Merges are left out of a followed listing, which is what
+	// git log --follow -- <file> does as well.
 	if (parents && parents->next)
 		return 0;
 
-	/*
-	 * If this is the root commit, do what rev_info tells us.
-	 */
 	if (!parents)
 		return revs->show_root_diff;
 
-	/* When we get here we have precisely one parent. */
 	parent = parents->item;
-	/* If we can't parse the commit, let print_commit() report an error. */
+
+	// A parent that does not parse is left for print_commit to report on,
+	// so the commit is shown.
 	if (repo_parse_commit(the_repository, parent))
 		return 1;
 
-	files = 0;
-	add_lines = 0;
-	rem_lines = 0;
+	file_count = 0;
+	added_lines = 0;
+	removed_lines = 0;
 
 	revs->diffopt.flags.recursive = 1;
 	diff_tree_oid(get_commit_tree_oid(parent),
@@ -166,16 +131,88 @@ static int show_commit(struct commit *commit, struct rev_info *revs)
 	saved_fmt = revs->diffopt.output_format;
 	revs->diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	revs->diffopt.format_callback = cgit_diff_tree_cb;
-	revs->diffopt.format_callback_data = handle_rename;
+	revs->diffopt.format_callback_data = follow_path;
 	revs->diffopt.no_free = 1;
 	diff_flush(&revs->diffopt);
 	revs->diffopt.output_format = saved_fmt;
 	revs->diffopt.flags = saved_flags;
 
-	lines_counted = 1;
+	counts_ready = 1;
 	return found;
 }
 
+/*
+ * Draws the graph lines that belong above a commit. The line that carries the
+ * commit itself stays in graph for the caller to place in the commit's own
+ * row.
+ */
+static void print_graph_above(struct rev_info *revs, struct strbuf *graph,
+			      int columns)
+{
+	while (!graph_next_line(revs->graph, graph)) {
+		html("<tr class='nohover'><td class='commitgraph'>");
+		html(graph->buf);
+		htmlf("</td><td colspan='%d' /></tr>\n", columns);
+		strbuf_setlen(graph, 0);
+	}
+}
+
+/*
+ * Cuts a subject longer than max-message-len down to size, leaving an arrow
+ * where it was cut and moving the remainder to the front of msg. The subject
+ * is edited in place, which the caller allows by freeing the commitinfo once
+ * the row is written.
+ */
+static void wrap_subject(struct commitinfo *info, struct strbuf *msg)
+{
+	// The arrow is U+21B5 written out in PAGE_ENCODING.
+	const char wrap_symbol[] = { ' ', 0xE2, 0x86, 0xB5, 0 };
+	size_t subject_len = strlen(info->subject);
+	int cut;
+
+	if (ctx.cfg.max_msg_len < MIN_WRAP_LEN ||
+	    subject_len <= (size_t)ctx.cfg.max_msg_len)
+		return;
+
+	cut = ctx.cfg.max_msg_len - strlen(wrap_symbol);
+
+	while (cut > 0 && !isspace((unsigned char)info->subject[cut]))
+		--cut;
+	if (!cut)
+		cut = ctx.cfg.max_msg_len - strlen(wrap_symbol);
+
+	strbuf_add(msg, info->subject + cut, subject_len - cut);
+	strbuf_trim(msg);
+	strbuf_add(msg, "\n\n", 2);
+	strlcpy(info->subject + cut, wrap_symbol, subject_len - cut + 1);
+}
+
+static int line_count(const char *text)
+{
+	int lines = 1;
+
+	while ((text = strchr(text, '\n'))) {
+		text++;
+		lines++;
+	}
+	return lines;
+}
+
+static void print_graph_padding(struct rev_info *revs, struct strbuf *graph,
+				int lines)
+{
+	html("<td class='commitgraph'>");
+	while (lines > 0 || !graph_is_commit_finished(revs->graph)) {
+		if (graph->len)
+			html("\n");
+		strbuf_setlen(graph, 0);
+		graph_next_line(revs->graph, graph);
+		html(graph->buf);
+		lines--;
+	}
+	html("</td>\n");
+}
+
 static void print_commit(struct commit *commit, struct rev_info *revs)
 {
 	struct commitinfo *info;
@@ -188,23 +225,13 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 	if (ctx.repo->enable_log_linecount)
 		columns++;
 
-	if (revs->graph) {
-		/* Advance graph until current commit */
-		while (!graph_next_line(revs->graph, &graphbuf)) {
-			/* Print graph segment in otherwise empty table row */
-			html("<tr class='nohover'><td class='commitgraph'>");
-			html(graphbuf.buf);
-			htmlf("</td><td colspan='%d' /></tr>\n", columns);
-			strbuf_setlen(&graphbuf, 0);
-		}
-		/* Current commit's graph segment is now ready in graphbuf */
-	}
+	if (revs->graph)
+		print_graph_above(revs, &graphbuf, columns);
 
 	info = cgit_parse_commit(commit);
 	htmlf("<tr%s>", ctx.qry.showmsg ? " class='logheader'" : "");
 
 	if (revs->graph) {
-		/* Print graph segment for current commit */
 		html("<td class='commitgraph'>");
 		html(graphbuf.buf);
 		html("</td>");
@@ -212,36 +239,14 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 	}
 	else {
 		html("<td>");
-		cgit_print_age(info->committer_date, info->committer_tz, TM_WEEK * 2);
+		cgit_print_age(info->committer_date, info->committer_tz,
+			       MAX_RELATIVE_AGE);
 		html("</td>");
 	}
 
 	htmlf("<td%s>", ctx.qry.showmsg ? " class='logsubject'" : "");
-	if (ctx.qry.showmsg) {
-		/* line-wrap long commit subjects instead of truncating them */
-		size_t subject_len = strlen(info->subject);
-
-		if (ctx.cfg.max_msg_len >= 15 &&
-		    subject_len > (size_t)ctx.cfg.max_msg_len) {
-			/* symbol for signaling line-wrap (in PAGE_ENCODING) */
-			const char wrap_symbol[] = { ' ', 0xE2, 0x86, 0xB5, 0 };
-			int i = ctx.cfg.max_msg_len - strlen(wrap_symbol);
-
-			/* Rewind i to preceding space character */
-			while (i > 0 && !isspace(info->subject[i]))
-				--i;
-			if (!i) /* Oops, zero spaces. Reset i */
-				i = ctx.cfg.max_msg_len - strlen(wrap_symbol);
-
-			/* add remainder starting at i to msgbuf */
-			strbuf_add(&msgbuf, info->subject + i, subject_len - i);
-			strbuf_trim(&msgbuf);
-			strbuf_add(&msgbuf, "\n\n", 2);
-
-			/* Place wrap_symbol at position i in info->subject */
-			strlcpy(info->subject + i, wrap_symbol, subject_len - i + 1);
-		}
-	}
+	if (ctx.qry.showmsg)
+		wrap_subject(info, &msgbuf);
 	cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
 			 oid_to_hex(&commit->object.oid), ctx.qry.vpath);
 	cgit_print_commit_decorations(commit);
@@ -252,31 +257,32 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 
 	if (revs->graph) {
 		html("</td><td>");
-		cgit_print_age(info->committer_date, info->committer_tz, TM_WEEK * 2);
+		cgit_print_age(info->committer_date, info->committer_tz,
+			       MAX_RELATIVE_AGE);
 	}
 
-	if (!lines_counted && (ctx.repo->enable_log_filecount ||
-			       ctx.repo->enable_log_linecount)) {
-		files = 0;
-		add_lines = 0;
-		rem_lines = 0;
-		cgit_diff_commit(commit, inspect_files, ctx.qry.vpath);
+	if (!counts_ready && (ctx.repo->enable_log_filecount ||
+			      ctx.repo->enable_log_linecount)) {
+		file_count = 0;
+		added_lines = 0;
+		removed_lines = 0;
+		cgit_diff_commit(commit, count_file, ctx.qry.vpath);
 	}
 
 	if (ctx.repo->enable_log_filecount)
-		htmlf("</td><td class='col-count'>%d", files);
+		htmlf("</td><td class='col-count'>%d", file_count);
 	if (ctx.repo->enable_log_linecount)
 		htmlf("</td><td class='col-count'><span class='deletions'>-%d</span>/"
-			"<span class='insertions'>+%d</span>", rem_lines, add_lines);
+			"<span class='insertions'>+%d</span>",
+			removed_lines, added_lines);
 
 	html("</td></tr>\n");
 
 	if ((revs->graph && !graph_is_commit_finished(revs->graph))
-			|| ctx.qry.showmsg) { /* Print a second table row */
+			|| ctx.qry.showmsg) {
 		html("<tr class='nohover-highlight'>");
 
 		if (ctx.qry.showmsg) {
-			/* Concatenate commit message + notes in msgbuf */
 			if (info->msg && *(info->msg)) {
 				strbuf_addstr(&msgbuf, info->msg);
 				strbuf_addch(&msgbuf, '\n');
@@ -287,36 +293,16 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 			strbuf_ltrim(&msgbuf);
 		}
 
+		// With no graph the empty cell stands in for the age column.
 		if (revs->graph) {
-			int lines = 0;
-
-			/* Calculate graph padding */
-			if (ctx.qry.showmsg) {
-				/* Count #lines in commit message + notes */
-				const char *p = msgbuf.buf;
-				lines = 1;
-				while ((p = strchr(p, '\n'))) {
-					p++;
-					lines++;
-				}
-			}
+			int msg_lines = ctx.qry.showmsg ?
+					line_count(msgbuf.buf) : 0;
 
-			/* Print graph padding */
-			html("<td class='commitgraph'>");
-			while (lines > 0 || !graph_is_commit_finished(revs->graph)) {
-				if (graphbuf.len)
-					html("\n");
-				strbuf_setlen(&graphbuf, 0);
-				graph_next_line(revs->graph, &graphbuf);
-				html(graphbuf.buf);
-				lines--;
-			}
-			html("</td>\n");
+			print_graph_padding(revs, &graphbuf, msg_lines);
 		}
 		else
-			html("<td/>"); /* Empty 'Age' column */
+			html("<td/>");
 
-		/* Print msgbuf into remainder of table row */
 		htmlf("<td colspan='%d'%s>\n", columns - (revs->graph ? 1 : 0),
 			ctx.qry.showmsg ? " class='logmsg'" : "");
 		html_txt(msgbuf.buf);
@@ -346,7 +332,7 @@ static const char *disambiguate_ref(const char *ref, int *must_free_result)
 
 static char *next_token(char **src)
 {
-	char *result;
+	char *token;
 
 	if (!src || !*src)
 		return NULL;
@@ -354,7 +340,7 @@ static char *next_token(char **src)
 		(*src)++;
 	if (!**src)
 		return NULL;
-	result = *src;
+	token = *src;
 	while (**src) {
 		if (isspace((unsigned char)**src)) {
 			**src = '\0';
@@ -363,11 +349,97 @@ static char *next_token(char **src)
 		}
 		(*src)++;
 	}
-	return result;
+	return token;
+}
+
+/*
+ * Whether there is a page after this one is answered by pulling one more
+ * commit off the walk.
+ */
+static void print_pager(struct rev_info *revs, int ofs, int cnt)
+{
+	struct commit *commit;
+
+	html("</table><ul class='pager'>");
+	if (ofs > 0) {
+		html("<li>");
+		cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
+			      ctx.qry.oid, ctx.qry.vpath,
+			      ofs - cnt, ctx.qry.grep,
+			      ctx.qry.search, ctx.qry.showmsg,
+			      ctx.qry.follow);
+		html("</li>");
+	}
+	if ((commit = get_revision(revs)) != NULL) {
+		html("<li>");
+		cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
+			      ctx.qry.oid, ctx.qry.vpath,
+			      ofs + cnt, ctx.qry.grep,
+			      ctx.qry.search, ctx.qry.showmsg,
+			      ctx.qry.follow);
+		html("</li>");
+	}
+	html("</ul>");
+}
+
+/*
+ * The caller has to have loaded the ref decorations for the walk, since
+ * without them a commit carries none.
+ */
+void cgit_print_commit_decorations(struct commit *commit)
+{
+	const struct name_decoration *deco;
+	static char buf[1024];
+
+	buf[sizeof(buf) - 1] = 0;
+	deco = get_name_decoration(&commit->object);
+	if (!deco)
+		return;
+	html("<span class='decoration'>");
+	while (deco) {
+		struct object_id oid_tag, peeled;
+		int is_annotated = 0;
+
+		strlcpy(buf, prettify_refname(deco->name), sizeof(buf));
+		switch (deco->type) {
+		case DECORATION_NONE:
+			break;
+		case DECORATION_REF_LOCAL:
+			cgit_log_link(buf, NULL, "branch-deco", buf, NULL,
+				ctx.qry.vpath, 0, NULL, NULL,
+				ctx.qry.showmsg, 0);
+			break;
+		case DECORATION_REF_TAG:
+			if (!refs_read_ref(get_main_ref_store(the_repository),
+					   deco->name, &oid_tag) &&
+			    !peel_object(the_repository, &oid_tag, &peeled,
+					 PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE))
+				is_annotated = !oideq(&oid_tag, &peeled);
+			cgit_tag_link(buf, NULL, is_annotated ?
+				      "tag-annotated-deco" : "tag-deco", buf);
+			break;
+		case DECORATION_REF_REMOTE:
+			if (!ctx.repo->enable_remote_branches)
+				break;
+			cgit_log_link(buf, NULL, "remote-deco", NULL,
+				oid_to_hex(&commit->object.oid),
+				ctx.qry.vpath, 0, NULL, NULL,
+				ctx.qry.showmsg, 0);
+			break;
+		default:
+			cgit_commit_link(buf, NULL, "deco", ctx.qry.head,
+					oid_to_hex(&commit->object.oid),
+					ctx.qry.vpath);
+			break;
+		}
+		deco = deco->next;
+	}
+	html("</span>");
 }
 
-void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern,
-		    const char *path, int pager, int commit_graph, int commit_sort)
+void cgit_print_log(const char *tip, int ofs, int cnt, char *grep,
+		    char *pattern, const char *path, int pager,
+		    int commit_graph, int commit_sort)
 {
 	struct rev_info rev;
 	struct commit *commit;
@@ -375,18 +447,19 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	int i, columns = commit_graph ? 4 : 3;
 	int must_free_tip = 0;
 
-	/* rev_argv.argv[0] will be ignored by setup_revisions */
+	// setup_revisions skips the first entry the way a command skips its
+	// own name, so this one is only a placeholder.
 	strvec_push(&rev_argv, "log_rev_setup");
 
 	if (!tip)
 		tip = ctx.qry.head;
 	tip = disambiguate_ref(tip, &must_free_tip);
 	if (tip && tip[0] == '-') {
-		/* setup_revisions() parses a leading-dash argument as an
-		 * option, so a tip like the id= value "--output=<path>" would
-		 * be handled by git as a request to write an arbitrary file.
-		 * No valid ref or object name begins with a dash, so refuse it.
-		 */
+		// setup_revisions() reads a leading-dash argument as an
+		// option, so a tip arriving as the id= value "--output=<path>"
+		// would be handled by git as a request to write an arbitrary
+		// file. No valid ref or object name begins with a dash, so
+		// refuse it.
 		cgit_print_error_page(400, "Bad request", "Invalid revision");
 		if (must_free_tip)
 			free((char *)tip);
@@ -402,11 +475,12 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 			strvec_pushf(&rev_argv, "--%s=%s", grep, pattern);
 		} else if (!strcmp(grep, "range")) {
 			char *arg;
-			/* Split the pattern at whitespace and add each token
-			 * as a revision expression. Do not accept other
-			 * rev-list options. Also, replace the previously
-			 * pushed tip (it's no longer relevant).
-			 */
+
+			// Each whitespace separated token is taken as a
+			// revision expression and nothing else, since anything
+			// starting with a dash would reach setup_revisions as
+			// a rev-list option. The tip pushed above goes away
+			// because the range supersedes it.
 			strvec_pop(&rev_argv);
 			while ((arg = next_token(&pattern))) {
 				if (*arg == '-') {
@@ -419,14 +493,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		}
 	}
 
-	if (!path || !ctx.repo->enable_follow_links) {
-		/*
-		 * If we don't have a path, "follow" is a no-op so make sure
-		 * the variable is set to false to avoid needing to check
-		 * both this and whether we have a path everywhere.
-		 */
+	if (!path || !ctx.repo->enable_follow_links)
 		ctx.qry.follow = 0;
-	}
 
 	if (commit_graph && !ctx.qry.follow) {
 		strvec_push(&rev_argv, "--graph");
@@ -499,30 +567,21 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 	}
 	html("</tr>\n");
 
-	if (ofs<0)
+	if (ofs < 0)
 		ofs = 0;
 
-	for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; /* nop */) {
-		if (show_commit(commit, &rev))
+	for (i = 0; i < ofs && (commit = get_revision(&rev)) != NULL; ) {
+		if (should_show(commit, &rev))
 			i++;
 		release_commit_memory(the_repository->parsed_objects, commit);
 		commit->parents = NULL;
 	}
 
-	for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; /* nop */) {
-		/*
-		 * In "follow" mode, we must count the files and lines the
-		 * first time we invoke diff on a given commit, and we need
-		 * to do that to see if the commit touches the path we care
-		 * about, so we do it in show_commit.  Hence we must clear
-		 * lines_counted here.
-		 *
-		 * This has the side effect of avoiding running diff twice
-		 * when we are both following renames and showing file
-		 * and/or line counts.
-		 */
-		lines_counted = 0;
-		if (show_commit(commit, &rev)) {
+	for (i = 0; i < cnt && (commit = get_revision(&rev)) != NULL; ) {
+		// Clearing the flag per commit keeps a commit from being
+		// diffed twice when the file or line columns are on.
+		counts_ready = 0;
+		if (should_show(commit, &rev)) {
 			i++;
 			print_commit(commit, &rev);
 		}
@@ -530,26 +589,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		commit->parents = NULL;
 	}
 	if (pager) {
-		html("</table><ul class='pager'>");
-		if (ofs > 0) {
-			html("<li>");
-			cgit_log_link("[prev]", NULL, NULL, ctx.qry.head,
-				      ctx.qry.oid, ctx.qry.vpath,
-				      ofs - cnt, ctx.qry.grep,
-				      ctx.qry.search, ctx.qry.showmsg,
-				      ctx.qry.follow);
-			html("</li>");
-		}
-		if ((commit = get_revision(&rev)) != NULL) {
-			html("<li>");
-			cgit_log_link("[next]", NULL, NULL, ctx.qry.head,
-				      ctx.qry.oid, ctx.qry.vpath,
-				      ofs + cnt, ctx.qry.grep,
-				      ctx.qry.search, ctx.qry.showmsg,
-				      ctx.qry.follow);
-			html("</li>");
-		}
-		html("</ul>");
+		print_pager(&rev, ofs, cnt);
 		cgit_print_layout_end();
 	} else if ((commit = get_revision(&rev)) != NULL) {
 		htmlf("<tr class='nohover'><td colspan='%d'>", columns);
@@ -559,7 +599,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
 		html("</td></tr>\n");
 	}
 
-	/* If we allocated tip then it is safe to cast away const. */
+	// The cast is safe because must_free_tip is only set for a string this
+	// function allocated.
 	if (must_free_tip)
-		free((char*) tip);
+		free((char *)tip);
 }