Restyle the sources and fix the audit's findings

1 file changed, 138 insertions, 110 deletions

diff --git a/source/ui-refs.c b/source/ui-refs.c
index 84b929c..d179cdf 100644
--- a/source/ui-refs.c
+++ b/source/ui-refs.c
@@ -1,33 +1,38 @@
-/* ui-refs.c: browse symbolic refs
- *
- * 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 branch and tag listings, shown as sections of the summary page and as
+ * the whole of a repository's refs page. Every row is one ref beside
+ * the commit or tag object it points at, ordered newest first, with branches
+ * then reordered by name unless branch-sort asks for age. A section longer
+ * than max-ref-count is cut short and ends in a link to a dedicated heads or
+ * tags page, which walks the same list one offset at a time.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
-#include "ui-refs.h"
+#include "filter.h"
 #include "html.h"
+#include "shared.h"
+#include "ui-refs.h"
 #include "ui-shared.h"
 
-static inline int cmp_age(int age1, int age2)
-{
-	/* age1 and age2 are assumed to be non-negative */
-	return age2 - age1;
-}
-
-static int cmp_ref_name(const void *a, const void *b)
-{
-	struct refinfo *r1 = *(struct refinfo **)a;
-	struct refinfo *r2 = *(struct refinfo **)b;
-
-	return strcmp(r1->refname, r2->refname);
-}
+/*
+ * The slice of a sorted ref list that one page shows, with end one past the
+ * last row. size is what a full page holds, so it also decides whether the
+ * list needs a pager.
+ */
+struct ref_page {
+	int size;
+	int start;
+	int end;
+};
 
-static int get_ref_age(struct refinfo *ref)
+/*
+ * The tagger date and the committer date live in a union in struct refinfo and
+ * only the member matching the object type is ever filled, so assuming a ref
+ * points at a commit reads past the end of the smaller struct.
+ */
+static timestamp_t ref_date(struct refinfo *ref)
 {
 	if (!ref->object)
 		return 0;
@@ -40,22 +45,53 @@ static int get_ref_age(struct refinfo *ref)
 	return 0;
 }
 
-// tag and commit share a union in struct refinfo and only the member matching
-// the object type is ever filled, so the date has to be reached through
-// get_ref_age rather than by assuming a branch points at a commit. Reading the
-// wrong member ran off the end of the smaller struct.
-static int cmp_ref_age(const void *a, const void *b)
+static int cmp_date(const void *a, const void *b)
+{
+	struct refinfo *ref1 = *(struct refinfo **)a;
+	struct refinfo *ref2 = *(struct refinfo **)b;
+	timestamp_t date1 = ref_date(ref1), date2 = ref_date(ref2);
+
+	// Compared rather than subtracted, at the width the dates are stored
+	// at. A commit may carry any timestamp, so a difference that does not
+	// fit an int would leave qsort with a contradictory ordering.
+	if (date1 < date2)
+		return 1;
+	if (date1 > date2)
+		return -1;
+	return 0;
+}
+
+static int cmp_name(const void *a, const void *b)
 {
-	struct refinfo *r1 = *(struct refinfo **)a;
-	struct refinfo *r2 = *(struct refinfo **)b;
+	struct refinfo *ref1 = *(struct refinfo **)a;
+	struct refinfo *ref2 = *(struct refinfo **)b;
+
+	return strcmp(ref1->refname, ref2->refname);
+}
+
+static void collect_branches(struct reflist *list)
+{
+	list->refs = NULL;
+	list->alloc = list->count = 0;
+	refs_for_each_branch_ref(get_main_ref_store(the_repository),
+				 cgit_refs_cb, list);
+	if (ctx.repo->enable_remote_branches)
+		refs_for_each_remote_ref(get_main_ref_store(the_repository),
+					 cgit_refs_cb, list);
+}
 
-	return cmp_age(get_ref_age(r1), get_ref_age(r2));
+static void print_branch_header(void)
+{
+	html("<tr class='nohover'><th class='left'>Branch</th>"
+	     "<th class='left'>Commit message</th>"
+	     "<th class='left col-author'>Author</th>"
+	     "<th class='left' colspan='2'>Age</th></tr>\n");
 }
 
 static int print_branch(struct refinfo *ref)
 {
 	struct commitinfo *info = ref->commit;
-	char *name = (char *)ref->refname;
+	const char *name = ref->refname;
 
 	if (!info)
 		return 1;
@@ -80,6 +116,14 @@ static int print_branch(struct refinfo *ref)
 	return 0;
 }
 
+static void collect_tags(struct reflist *list)
+{
+	list->refs = NULL;
+	list->alloc = list->count = 0;
+	refs_for_each_tag_ref(get_main_ref_store(the_repository),
+			      cgit_refs_cb, list);
+}
+
 static void print_tag_header(void)
 {
 	html("<tr class='nohover'><th class='left'>Tag</th>"
@@ -90,13 +134,15 @@ static void print_tag_header(void)
 
 static int print_tag(struct refinfo *ref)
 {
-	struct tag *tag = NULL;
 	struct taginfo *info = NULL;
-	char *name = (char *)ref->refname;
+	const char *name = ref->refname;
 	struct object *obj = ref->object;
 
+	// A lightweight tag has no tag object, so the author and age columns
+	// below fall back to the commit.
 	if (obj->type == OBJ_TAG) {
-		tag = (struct tag *)obj;
+		struct tag *tag = (struct tag *)obj;
+
 		obj = tag->tagged;
 		info = ref->tag;
 		if (!info)
@@ -141,16 +187,16 @@ static void print_refs_link(const char *path)
 	html("</td></tr>");
 }
 
-/* Prev/next links for the dedicated branch and tag pages, so each
- * category pages independently instead of sharing one endless page. */
 static void print_ref_pager(int ofs, int pagesize, int count, const char *path)
 {
 	char *url;
 
 	html("<tr class='nohover'><td colspan='5' class='refs-pager'>");
 	if (ofs > 0) {
+		int prev_ofs = ofs > pagesize ? ofs - pagesize : 0;
+
 		url = cgit_pageurl(ctx.qry.repo, cgit_fmt("refs/%s", path),
-				   cgit_fmt("ofs=%d", ofs > pagesize ? ofs - pagesize : 0));
+				   cgit_fmt("ofs=%d", prev_ofs));
 		html("<a href='");
 		html_attr(url);
 		html("'>[prev]</a> ");
@@ -169,132 +215,116 @@ static void print_ref_pager(int ofs, int pagesize, int count, const char *path)
 	html("</td></tr>");
 }
 
-static void collect_branches(struct reflist *list)
+static struct ref_page page_bounds(int pagesize, int count)
 {
-	list->refs = NULL;
-	list->alloc = list->count = 0;
-	refs_for_each_branch_ref(get_main_ref_store(the_repository),
-				 cgit_refs_cb, list);
-	if (ctx.repo->enable_remote_branches)
-		refs_for_each_remote_ref(get_main_ref_store(the_repository),
-					 cgit_refs_cb, list);
-}
+	struct ref_page page;
 
-static void print_branch_header(void)
-{
-	html("<tr class='nohover'><th class='left'>Branch</th>"
-	     "<th class='left'>Commit message</th>"
-	     "<th class='left col-author'>Author</th>"
-	     "<th class='left' colspan='2'>Age</th></tr>\n");
+	page.size = (pagesize <= 0 || pagesize > count) ? count : pagesize;
+	page.start = ctx.qry.ofs > 0 ? ctx.qry.ofs : 0;
+	if (page.start > count)
+		page.start = count;
+	page.end = page.start + page.size < count ?
+		   page.start + page.size : count;
+	return page;
 }
 
-void cgit_print_branches(int maxcount)
+/*
+ * Unlike the capped sections, the whole list is sorted before a page is cut
+ * out of it, so a branch keeps its place no matter which page it lands on.
+ */
+static void print_branches_page(int pagesize)
 {
 	struct reflist list;
+	struct ref_page page;
 	int i;
 
 	print_branch_header();
 	collect_branches(&list);
 
-	if (maxcount == 0 || maxcount > list.count)
-		maxcount = list.count;
-
-	qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age);
+	qsort(list.refs, list.count, sizeof(*list.refs), cmp_date);
 	if (ctx.repo->branch_sort == 0)
-		qsort(list.refs, maxcount, sizeof(*list.refs), cmp_ref_name);
+		qsort(list.refs, list.count, sizeof(*list.refs), cmp_name);
 
-	for (i = 0; i < maxcount; i++)
+	page = page_bounds(pagesize, list.count);
+
+	for (i = page.start; i < page.end; i++)
 		print_branch(list.refs[i]);
 
-	if (maxcount < list.count)
-		print_refs_link("heads");
+	if (page.size < list.count)
+		print_ref_pager(page.start, page.size, list.count, "heads");
 
 	cgit_free_reflist_inner(&list);
 }
 
-void cgit_print_tags(int maxcount)
+static void print_tags_page(int pagesize)
 {
 	struct reflist list;
+	struct ref_page page;
 	int i;
 
-	list.refs = NULL;
-	list.alloc = list.count = 0;
-	refs_for_each_tag_ref(get_main_ref_store(the_repository),
-			      cgit_refs_cb, &list);
+	collect_tags(&list);
 	if (list.count == 0)
 		return;
-	qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age);
-	if (!maxcount)
-		maxcount = list.count;
-	else if (maxcount > list.count)
-		maxcount = list.count;
+	qsort(list.refs, list.count, sizeof(*list.refs), cmp_date);
+
+	page = page_bounds(pagesize, list.count);
+
 	print_tag_header();
-	for (i = 0; i < maxcount; i++)
+	for (i = page.start; i < page.end; i++)
 		print_tag(list.refs[i]);
 
-	if (maxcount < list.count)
-		print_refs_link("tags");
+	if (page.size < list.count)
+		print_ref_pager(page.start, page.size, list.count, "tags");
 
 	cgit_free_reflist_inner(&list);
 }
 
-/* The dedicated branch page lists everything, a page at a time. The
- * whole list is name-sorted (or age-sorted per branch-sort) so the
- * order is stable across pages. */
-static void print_branches_page(int pagesize)
+void cgit_print_branches(int maxcount)
 {
 	struct reflist list;
-	int i, ofs, end;
+	int i;
 
 	print_branch_header();
 	collect_branches(&list);
 
-	qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age);
-	if (ctx.repo->branch_sort == 0)
-		qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_name);
+	if (maxcount == 0 || maxcount > list.count)
+		maxcount = list.count;
 
-	if (pagesize <= 0 || pagesize > list.count)
-		pagesize = list.count;
-	ofs = ctx.qry.ofs > 0 ? ctx.qry.ofs : 0;
-	if (ofs > list.count)
-		ofs = list.count;
-	end = ofs + pagesize < list.count ? ofs + pagesize : list.count;
+	// The date sort covers the list, the name sort only the rows about to
+	// be shown, so the section holds the newest branches rather than the
+	// first ones by name.
+	qsort(list.refs, list.count, sizeof(*list.refs), cmp_date);
+	if (ctx.repo->branch_sort == 0)
+		qsort(list.refs, maxcount, sizeof(*list.refs), cmp_name);
 
-	for (i = ofs; i < end; i++)
+	for (i = 0; i < maxcount; i++)
 		print_branch(list.refs[i]);
 
-	if (pagesize < list.count)
-		print_ref_pager(ofs, pagesize, list.count, "heads");
+	if (maxcount < list.count)
+		print_refs_link("heads");
 
 	cgit_free_reflist_inner(&list);
 }
 
-static void print_tags_page(int pagesize)
+void cgit_print_tags(int maxcount)
 {
 	struct reflist list;
-	int i, ofs, end;
+	int i;
 
-	list.refs = NULL;
-	list.alloc = list.count = 0;
-	refs_for_each_tag_ref(get_main_ref_store(the_repository),
-			      cgit_refs_cb, &list);
+	collect_tags(&list);
 	if (list.count == 0)
 		return;
-	qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age);
-
-	if (pagesize <= 0 || pagesize > list.count)
-		pagesize = list.count;
-	ofs = ctx.qry.ofs > 0 ? ctx.qry.ofs : 0;
-	if (ofs > list.count)
-		ofs = list.count;
-	end = ofs + pagesize < list.count ? ofs + pagesize : list.count;
-
+	qsort(list.refs, list.count, sizeof(*list.refs), cmp_date);
+	if (!maxcount)
+		maxcount = list.count;
+	else if (maxcount > list.count)
+		maxcount = list.count;
 	print_tag_header();
-	for (i = ofs; i < end; i++)
+	for (i = 0; i < maxcount; i++)
 		print_tag(list.refs[i]);
 
-	if (pagesize < list.count)
-		print_ref_pager(ofs, pagesize, list.count, "tags");
+	if (maxcount < list.count)
+		print_refs_link("tags");
 
 	cgit_free_reflist_inner(&list);
 }
@@ -309,8 +339,6 @@ void cgit_print_refs(void)
 	else if (ctx.qry.path && starts_with(ctx.qry.path, "tags"))
 		print_tags_page(ctx.cfg.max_ref_count);
 	else {
-		/* The combined page caps each section, with the [...] rows
-		 * leading to the dedicated pages above. */
 		cgit_print_branches(ctx.cfg.max_ref_count);
 		html("<tr class='nohover'><td colspan='5'>&nbsp;</td></tr>");
 		cgit_print_tags(ctx.cfg.max_ref_count);