Trim the comments and dead code across the tree

29 files changed, 226 insertions, 318 deletions

diff --git a/source/cache.c b/source/cache.c
index dd8ae7f..2d546bb 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -96,9 +96,8 @@ static int open_slot(struct cache_slot *slot)
 }
 
 /*
- * A key longer than the buffer above can never be read back by open_slot, so
- * a slot keyed on one would never match and every such request would
- * regenerate its page while still writing a slot nothing can use.
+ * A key longer than the buffer above can never be read back by open_slot,
+ * so a slot keyed on one would be written but never match.
  */
 static int key_fits_slot(const char *key)
 {
@@ -238,10 +237,8 @@ static int lock_slot(struct cache_slot *slot)
 	}
 	// The lock landed on whatever inode the path named at open. A holder
 	// finishing in between renames that inode into place as the live
-	// slot, so truncating it on the strength of the stale descriptor
-	// would tear down the page other requests are reading. Once the path
-	// is confirmed to still name this file the rename can no longer
-	// happen, because doing so takes the lock now held here.
+	// slot, and once the path is confirmed to still name this file that
+	// rename can no longer happen, because it takes the lock held here.
 	if (fstat(slot->lock_fd, &held) || stat(slot->lock_path, &named) ||
 	    held.st_ino != named.st_ino || held.st_dev != named.st_dev) {
 		close(slot->lock_fd);
@@ -291,10 +288,9 @@ void cache_abandon_fill(void)
 	slot_being_filled = NULL;
 	slot->abandoned = 1;
 
-	// The page is sitting in html.c's buffer and in stdio's, and both are
-	// emptied while stdout still points at the lock file, so the half
-	// rendered page goes into the file about to be removed rather than
-	// reaching the client ahead of whatever is written next.
+	// The page is sitting in html.c's buffer and in stdio's. Empty both
+	// while stdout still points at the lock file so the half rendered
+	// page never reaches the client.
 	html_flush();
 	fflush(stdout);
 
@@ -323,10 +319,9 @@ static int fill_slot(struct cache_slot *slot)
 	slot->fn();
 	slot_being_filled = NULL;
 
-	// The page is sitting in html.c's buffer and then in stdio's, and all
-	// of it has to reach the lock file before that file is renamed into
-	// place. After an abandoned fill stdout is the client again and this
-	// same flush delivers the tail of the error page instead.
+	// All of the page has to reach the lock file before it is renamed
+	// into place. After an abandoned fill stdout is the client again and
+	// this same flush delivers the tail of the error page instead.
 	html_flush();
 	if (fflush(stdout))
 		return errno;
@@ -352,9 +347,7 @@ static void refresh_slot(struct cache_slot *slot)
 		return;
 
 	// If another process replaced the slot between open_slot and
-	// lock_slot, the copy already open is served rather than the newer
-	// one, which would mean opening that file and comparing the key in it,
-	// not worth a second descriptor and read on every expiry.
+	// lock_slot, the copy already open is served rather than the newer.
 	if (is_modified(slot) || fill_slot(slot)) {
 		unlock_slot(slot, 0);
 		close_lock(slot);
@@ -391,8 +384,7 @@ static int process_slot(struct cache_slot *slot)
 
 	// A slot that opened cleanly but holds another key is a collision,
 	// and two popular pages sharing one slot evict each other on every
-	// alternating visit. Nothing else makes that visible, because the
-	// cache keeps working and only quietly stops helping.
+	// alternating visit.
 	if (!err)
 		log_error("[cgit] Cache slot %s holds a different key, "
 			  "consider a larger cache-size\n", slot->path);
@@ -449,10 +441,8 @@ static char *format_time(const char *format, time_t when)
 }
 
 /*
- * The accumulator is an unsigned long rather than a fixed 32 bit type, so on a
- * 64 bit host this is not the published FNV-1 value. All that decides is which
- * slot a key lands in, and nothing outside a single build has to agree on the
- * answer.
+ * The accumulator is unsigned long, so on a 64 bit host this is not the
+ * published FNV-1 value. Only slot selection depends on it.
  */
 unsigned long cache_hash_str(const char *str)
 {
diff --git a/source/cgit.c b/source/cgit.c
index 730e2c6..dc56a0e 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -59,12 +59,10 @@ struct refmatch {
 const char *cgit_version = CGIT_VERSION;
 
 /*
- * Isolate git from the calling user's configuration, so a snapshot cannot be
- * broken by something like a core.excludesfile pointing at a "~" path that git
- * can no longer expand once HOME is unset below. Called from cmd_main rather
- * than from a constructor attribute, because git-compat-util.h defines
- * __attribute__ away on a compiler that does not support it, which would leave
- * this silently never running.
+ * Isolate git from the calling user's configuration, which could otherwise
+ * break once HOME is unset below. Called from cmd_main rather than from a
+ * constructor attribute, because git-compat-util.h defines __attribute__
+ * away on a compiler without it, which would leave this silently unrun.
  */
 static void isolate_git_environment(void)
 {
@@ -173,7 +171,8 @@ static void prepare_context(void)
 
 static void print_version(void)
 {
-	printf("CGit %s | https://github.com/brycekwon/cgit\n\nCompiled in features:\n", CGIT_VERSION);
+	printf("CGit %s | https://github.com/brycekwon/cgit\n\n"
+	       "Compiled in features:\n", CGIT_VERSION);
 #ifdef NO_LUA
 	printf("[-] ");
 #else
@@ -319,10 +318,9 @@ static void parse_args(int argc, const char **argv)
 			ctx.qry.ofs = atoi(arg);
 		} else if (skip_prefix(argv[i], "--scan-tree=", &arg) ||
 		           skip_prefix(argv[i], "--scan-path=", &arg)) {
-			// A repository's own snapshots setting is masked with
-			// the global one, which normally comes from cgitrc.
-			// That has not been read yet here, so an empty mask
-			// would discard whatever the repository asked for.
+			// A repository's snapshots setting is masked with the
+			// global one, and cgitrc has not been read here, so an
+			// empty mask would discard what the repository set.
 			ctx.cfg.snapshots = ALL_SNAPSHOT_FORMATS;
 			scanned++;
 			scan_tree(arg);
@@ -338,9 +336,8 @@ static void parse_args(int argc, const char **argv)
 
 /*
  * The lock is a fcntl lock rather than the mere existence of the lock file,
- * because a lock the kernel drops with its process cannot outlive a scan that
- * was killed mid-run. A leftover lock file used to count as a scan in
- * progress, and one crash would silently freeze the repolist for good.
+ * because a lock the kernel drops with its process cannot outlive a scan
+ * killed mid-run. A leftover file would count as a scan forever in progress.
  */
 static int generate_cached_repolist(const char *path, const char *cached_rc)
 {
@@ -379,10 +376,8 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	}
 	// The lock landed on whatever inode the path named at open. A holder
 	// finishing in between renames that inode into place as the live
-	// list, so truncating it on the strength of the stale descriptor
-	// would tear down the list other requests are reading. Once the path
-	// is confirmed to still name this file the rename can no longer
-	// happen, because doing so takes the lock now held here.
+	// list, and once the path is confirmed to still name this file that
+	// rename can no longer happen, because it takes the lock held here.
 	if (fstat(fd, &held) || stat(locked_rc.buf, &named) ||
 	    held.st_ino != named.st_ino || held.st_dev != named.st_dev) {
 		err = EAGAIN;
@@ -469,11 +464,10 @@ static void process_cached_repolist(const char *path)
 	if (fork())
 		goto out;
 
-	// The child inherits the descriptors of the request, and the web server
-	// reads stdout until every holder of it is gone, so leaving them in
-	// place would keep the visitor waiting for the whole scan after their
-	// page was written. Anything the scan prints would land on that
-	// response as well.
+	// The child inherits the request's descriptors, and the web server
+	// reads stdout until every holder is gone, so left in place they
+	// would keep the visitor waiting on the scan and let its output
+	// land on the response.
 	devnull = open("/dev/null", O_RDWR);
 	if (devnull >= 0) {
 		dup2(devnull, STDIN_FILENO);
@@ -703,9 +697,8 @@ static void apply_config(const char *name, const char *value)
 
 /*
  * Read a whole number a request supplied, clamped into the range the caller
- * accepts. strtol rather than atoi, because atoi has no defined behaviour once
- * the digits overflow and every value here arrives straight from the query
- * string.
+ * accepts. strtol rather than atoi, because atoi is undefined on overflow
+ * and every value here arrives straight from the query string.
  */
 static int query_int(const char *value, int min, int max)
 {
@@ -729,10 +722,9 @@ static void apply_query_param(const char *name, const char *value)
 	} else if (!strcmp(name, "p")) {
 		ctx.qry.page = xstrdup(value);
 	} else if (!strcmp(name, "url")) {
-		// Every leading slash goes, not just one. What is left is
-		// joined onto the virtual root, so a value like //example.com
-		// would otherwise survive as /example.com and make that join a
-		// scheme-relative link to another host.
+		// Every leading slash goes, not just one, so a value like
+		// //example.com cannot survive as /example.com and make the
+		// virtual root join a scheme-relative link to another host.
 		while (*value == '/')
 			value++;
 		ctx.qry.url = xstrdup(value);
@@ -754,11 +746,9 @@ static void apply_query_param(const char *name, const char *value)
 		ctx.qry.oid2 = xstrdup(value);
 		ctx.qry.has_oid = 1;
 	} else if (!strcmp(name, "ofs")) {
-		// Bounded above so a crafted value cannot force a walk over the
-		// whole history. Negatives stop at -1 rather than at zero,
-		// because the offset is overloaded, the stats page submits -1
-		// for all authors, and the log skip loop floors a negative
-		// itself.
+		// Bounded above so a crafted value cannot force a walk over
+		// the whole history. The floor is -1 rather than 0 because
+		// the stats page submits -1 for all authors.
 		ctx.qry.ofs = query_int(value, -1, MAX_QUERY_OFFSET);
 	} else if (!strcmp(name, "path")) {
 		ctx.qry.path = cgit_trim_end(value, '/');
@@ -847,10 +837,9 @@ static void authenticate_cookie(void)
 }
 
 /*
- * Only a full object id names content that can never change. The id parameter
- * accepts anything git can resolve, so a ref name or an abbreviation arrives
- * here just as marked as a real id, and a page pinned to one of those must
- * not be cached under the never-expiring static ttl.
+ * Only a full object id names content that can never change. The id
+ * parameter accepts anything git can resolve, so a page pinned to a ref
+ * name or abbreviation must not be cached under the static ttl.
  */
 static int is_full_oid(const char *rev)
 {
@@ -895,10 +884,9 @@ static int calc_ttl(void)
 }
 
 /*
- * The scheme and the host are folded in because the absolute urls a page
- * carries, its clone urls and atom links, are built from them, so a request
- * arriving with a spoofed Host must not poison the page served to a visitor
- * who came in on the real one.
+ * The scheme and host are folded in because the page's absolute urls, its
+ * clone urls and atom links, are built from them, so a spoofed Host must
+ * not poison the page served on the real one.
  */
 static void build_cache_key(struct strbuf *key)
 {
@@ -911,11 +899,9 @@ static void build_cache_key(struct strbuf *key)
 	};
 	size_t i;
 
-	// Each part is written behind its own length, so nothing a value
-	// contains can make two different requests spell one key. The path and
-	// the query come from the environment rather than the query string cgit
-	// rebuilds, since that rebuild folds the two together and would let the
-	// PATH_INFO and QUERY_STRING forms of one request share a slot.
+	// Each part is written behind its own length so no value can make two
+	// requests spell one key, and the path and query come from the
+	// environment because cgit's rebuilt query string folds them together.
 	for (i = 0; i < ARRAY_SIZE(parts); i++)
 		strbuf_addf(key, "%zu|%s", strlen(parts[i]), parts[i]);
 	free(hosturl);
@@ -1013,7 +999,6 @@ static void choose_readme(struct cgit_repo *repo)
 	for_each_string_list_item(entry, &repo->readme) {
 		parse_readme(entry->string, &filename, &ref, repo);
 		if (!filename) {
-			free(filename);
 			free(ref);
 			continue;
 		}
@@ -1041,8 +1026,6 @@ static void prepare_repo_env(int *nongit)
 {
 	setenv("GIT_DIR", ctx.repo->path, 1);
 
-	// Both read configuration out of the repository, with the user's own
-	// git configuration already stripped by isolate_git_environment.
 	setup_git_directory_gently(the_repository, nongit);
 	load_display_notes(NULL);
 }
@@ -1230,7 +1213,8 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu
 	else if (!strcmp(name, "about-filter") || !strcmp(name, "commit-filter") ||
 		 !strcmp(name, "source-filter") || !strcmp(name, "email-filter")) {
 		if (!ctx.cfg.enable_filter_overrides)
-			fprintf(stderr, "[cgit] Ignoring repo %s: enable-filter-overrides is not set\n", name);
+			fprintf(stderr, "[cgit] Ignoring repo %s: "
+				"enable-filter-overrides is not set\n", name);
 		else if (!strcmp(name, "about-filter"))
 			repo->about_filter = cgit_new_filter(value, ABOUT);
 		else if (!strcmp(name, "commit-filter"))
@@ -1275,8 +1259,7 @@ int cmd_main(int argc, const char **argv)
 	// string keeps it part of the cache key.
 	path = ctx.env.path_info;
 	if (!ctx.qry.url && path) {
-		// Stripped like the url parameter and for the same reason, so
-		// a request for //example.com cannot turn into a link off site.
+		// Stripped like the url parameter and for the same reason.
 		while (*path == '/')
 			path++;
 		ctx.qry.url = xstrdup(path);
diff --git a/source/cgit.h b/source/cgit.h
index 8d2d686..308fda2 100644
--- a/source/cgit.h
+++ b/source/cgit.h
@@ -50,13 +50,12 @@
 // converted on the way in.
 #define PAGE_ENCODING "UTF-8"
 
-// The month is a double because a twelfth of a year is not a whole number of
-// seconds.
 #define SECONDS_PER_MINUTE 60
 #define SECONDS_PER_HOUR   (SECONDS_PER_MINUTE * 60)
 #define SECONDS_PER_DAY    (SECONDS_PER_HOUR * 24)
 #define SECONDS_PER_WEEK   (SECONDS_PER_DAY * 7)
 #define SECONDS_PER_YEAR   (SECONDS_PER_DAY * 365)
+// A double, because a twelfth of a year is not a whole number of seconds.
 #define SECONDS_PER_MONTH  (SECONDS_PER_YEAR / 12.0)
 
 typedef enum {
diff --git a/source/cgit.mk b/source/cgit.mk
index 0683cb7..1106b64 100644
--- a/source/cgit.mk
+++ b/source/cgit.mk
@@ -9,10 +9,10 @@
 # ../../build.
 include Makefile
 
-# TOOLSDIR and BUILDDIR are named relative to the project root, matching the
-# top level Makefile, because the version recipe changes into the root before
-# using them. CGIT_SRC and CGIT_BUILD lead to the sources and the output from
-# vendor/git, where everything else here runs.
+# TOOLSDIR and BUILDDIR are named relative to the project root because the
+# version recipe changes into the root before using them. CGIT_SRC and
+# CGIT_BUILD lead to the sources and the output from vendor/git, where
+# everything else here runs.
 CGIT_ROOT  = ../..
 SRCDIR     = source
 TOOLSDIR   = tools
@@ -24,8 +24,7 @@ CGIT_BUILD = $(CGIT_ROOT)/$(BUILDDIR)
 # Makefile exports, which leaves out the build options this file reads.
 -include $(CGIT_ROOT)/cgit.conf
 
-# CGIT_VERSION and the other CGIT_ values used below come from the top level
-# Makefile, which exports them, rather than being defined in this file.
+# The CGIT_ values used below are exported by the top level Makefile.
 $(CGIT_BUILD)/VERSION: force-version
 	@mkdir -p $(CGIT_BUILD)/
 	@cd $(CGIT_ROOT) && '$(SHELL_PATH_SQ)' $(TOOLSDIR)/gen-version.sh "$(CGIT_VERSION)" $(BUILDDIR)/VERSION
@@ -42,15 +41,16 @@ $(CGIT_BUILD)/VERSION: force-version
 # standard again.
 CGIT_STD ?= gnu17
 
-# CGIT_CFLAGS is tracked separately so that changing it does not force a
-# rebuild of Git itself.
+# CGIT_CFLAGS is tracked separately so that changing it does not force a rebuild
+# of Git itself.
 CGIT_CFLAGS += -std=$(CGIT_STD)
 CGIT_CFLAGS += -DCGIT_CONFIG='"$(CGIT_CONFIG)"'
 CGIT_CFLAGS += -DCGIT_SCRIPT_NAME='"$(CGIT_SCRIPT_NAME)"'
 CGIT_CFLAGS += -DCGIT_CACHE_ROOT='"$(CACHE_ROOT)"'
 
-# Reaches only the cgit objects, so a caller can tighten the build, the way CI
-# passes -Werror, without holding git's own sources to the same standard.
+# Reaches only the cgit objects, so a caller can tighten the build, the way the
+# release build passes -Werror=format-security, without holding git's own
+# sources to the same standard.
 CGIT_CFLAGS += $(CGIT_EXTRA_CFLAGS)
 
 PKG_CONFIG ?= pkg-config
@@ -161,10 +161,12 @@ $(CGIT_BUILD)/CGIT-CFLAGS: FORCE
 	    if test x"$$FLAGS" != x"`cat $(CGIT_BUILD)/CGIT-CFLAGS 2>/dev/null`" ; then \
 		echo 1>&2 "    * new CGit build flags"; \
 		echo "$$FLAGS" >$(CGIT_BUILD)/CGIT-CFLAGS; \
-            fi
+	    fi
 
-$(CGIT_OBJS): $(CGIT_BUILD)/%.o: $(CGIT_SRC)/%.c GIT-CFLAGS $(CGIT_BUILD)/CGIT-CFLAGS $(missing_dep_dirs)
-	$(QUIET_CC)$(CC) -o $@ -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $(CGIT_CFLAGS) $<
+$(CGIT_OBJS): $(CGIT_BUILD)/%.o: $(CGIT_SRC)/%.c GIT-CFLAGS \
+		$(CGIT_BUILD)/CGIT-CFLAGS $(missing_dep_dirs)
+	$(QUIET_CC)$(CC) -o $@ -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
+		$(CGIT_CFLAGS) $<
 
 $(CGIT_BUILD)/cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS)
 	@echo 1>&2 "    * $(LUA_MESSAGE)"
diff --git a/source/config.c b/source/config.c
index 6a5d136..0ff8588 100644
--- a/source/config.c
+++ b/source/config.c
@@ -1,12 +1,9 @@
 /*
- * The reader for cgit's config files, which are the main cgitrc, any file it
- * pulls in with an include line, the cgitrc that sits beside a scanned
- * repository, and the cached repolist. A file is a sequence of name=value
- * lines, and each pair is handed to a callback that decides what it means, so
- * nothing here knows a single key by name. A value runs to the end of its line
- * and keeps whatever spacing it has, there is no quoting. A line whose first
- * non blank character is a hash or a semicolon is a comment, and blank lines
- * are ignored.
+ * The reader for cgit's config files, which are the main cgitrc, any file
+ * it pulls in with an include line, the cgitrc beside a scanned repository,
+ * and the cached repolist. A file is a sequence of name=value lines, each
+ * handed to a callback that decides what it means, so nothing here knows a
+ * single key by name.
  */
 
 #include "cgit.h"
@@ -95,9 +92,8 @@ int config_file_parse(const char *filename, config_file_value_fn fn)
 	struct strbuf value = STRBUF_INIT;
 	FILE *f;
 
-	// An include line calls back into here, so a file that includes itself,
-	// directly or round a longer loop, would recurse until the stack gave
-	// out.
+	// An include line calls back into here, so an include cycle would
+	// recurse until the stack gave out.
 	if (nesting > MAX_INCLUDE_NESTING)
 		return -1;
 	if (!(f = fopen(filename, "r")))
diff --git a/source/filter.c b/source/filter.c
index 3c0c7e4..f5f971c 100644
--- a/source/filter.c
+++ b/source/filter.c
@@ -78,14 +78,10 @@ static void fprintf_exec_filter(struct cgit_filter *base, FILE *f,
 static void cleanup_exec_filter(struct cgit_filter *base)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
-	if (filter->argv) {
-		free(filter->argv);
-		filter->argv = NULL;
-	}
-	if (filter->cmd) {
-		free(filter->cmd);
-		filter->cmd = NULL;
-	}
+	free(filter->argv);
+	filter->argv = NULL;
+	free(filter->cmd);
+	filter->cmd = NULL;
 }
 
 static struct cgit_filter *new_exec_filter(const char *cmd, int argument_count)
@@ -175,11 +171,10 @@ static inline void unhook_write(void)
 	current_write_filter = NULL;
 }
 
-static void die_lua_error(struct lua_filter *filter)
+static NORETURN void die_lua_error(struct lua_filter *filter)
 {
 	die("Lua error in %s: %s", filter->script_file,
 	    lua_tostring(filter->lua_state, -1));
-	lua_pop(filter->lua_state, 1);
 }
 
 static ssize_t write_lua_filter(struct cgit_filter *base, const void *buf,
@@ -189,18 +184,15 @@ static ssize_t write_lua_filter(struct cgit_filter *base, const void *buf,
 
 	lua_getglobal(filter->lua_state, "filter_write");
 	lua_pushlstring(filter->lua_state, buf, count);
-	if (lua_pcall(filter->lua_state, 1, 0, 0)) {
+	if (lua_pcall(filter->lua_state, 1, 0, 0))
 		die_lua_error(filter);
-		errno = EIO;
-		return -1;
-	}
 	return count;
 }
 
 /*
- * Output a script asks for belongs on the page and not back in its own filter,
- * so the hook comes off around the call. The zero returned is Lua's count of
- * values pushed for the script, not a success code.
+ * Output a script asks for belongs on the page, not back in its own filter,
+ * so the hook comes off around the call. The zero returned is Lua's count
+ * of values pushed, not a success code.
  */
 static inline int emit_unfiltered(lua_State *lua_state,
 				  void (*emit)(const char *text))
@@ -295,12 +287,8 @@ static int init_lua_filter(struct lua_filter *filter)
 		lua_setglobal(filter->lua_state, script_globals[i].name);
 	}
 
-	if (luaL_dofile(filter->lua_state, filter->script_file)) {
+	if (luaL_dofile(filter->lua_state, filter->script_file))
 		die_lua_error(filter);
-		lua_close(filter->lua_state);
-		filter->lua_state = NULL;
-		return 1;
-	}
 	return 0;
 }
 
@@ -317,26 +305,21 @@ static int open_lua_filter(struct cgit_filter *base, va_list ap)
 	lua_getglobal(filter->lua_state, "filter_open");
 	for (i = 0; i < filter->base.argument_count; ++i)
 		lua_pushstring(filter->lua_state, va_arg(ap, char *));
-	if (lua_pcall(filter->lua_state, filter->base.argument_count, 0, 0)) {
+	if (lua_pcall(filter->lua_state, filter->base.argument_count, 0, 0))
 		die_lua_error(filter);
-		return 1;
-	}
 	return 0;
 }
 
 static int close_lua_filter(struct cgit_filter *base)
 {
 	struct lua_filter *filter = (struct lua_filter *)base;
-	int ret = 0;
+	int ret;
 
 	lua_getglobal(filter->lua_state, "filter_close");
-	if (lua_pcall(filter->lua_state, 0, 1, 0)) {
+	if (lua_pcall(filter->lua_state, 0, 1, 0))
 		die_lua_error(filter);
-		ret = -1;
-	} else {
-		ret = lua_tonumber(filter->lua_state, -1);
-		lua_pop(filter->lua_state, 1);
-	}
+	ret = lua_tonumber(filter->lua_state, -1);
+	lua_pop(filter->lua_state, 1);
 
 	unhook_write();
 	return ret;
@@ -358,10 +341,8 @@ static void cleanup_lua_filter(struct cgit_filter *base)
 
 	lua_close(filter->lua_state);
 	filter->lua_state = NULL;
-	if (filter->script_file) {
-		free(filter->script_file);
-		filter->script_file = NULL;
-	}
+	free(filter->script_file);
+	filter->script_file = NULL;
 }
 
 static struct cgit_filter *new_lua_filter(const char *cmd, int argument_count)
diff --git a/source/html.c b/source/html.c
index 2e4ec66..2e3a3b7 100644
--- a/source/html.c
+++ b/source/html.c
@@ -3,10 +3,9 @@
  * for page text, attribute values, URL paths and query arguments along with
  * the small formatting helpers the rest of the code prints through. What is
  * written here is gathered into one buffer and handed to stdout in whole
- * blocks, because a page is made of a great many small fragments and a write
- * apiece spent more time in the kernel than rendering the page did. cgit
- * shares stdout with the filters it runs and with git itself, so that buffer
- * has to be emptied wherever another writer takes over.
+ * blocks, since a page is made of a great many small fragments. cgit shares
+ * stdout with the filters it runs and with git itself, so that buffer has
+ * to be emptied wherever another writer takes over.
  */
 
 #include "cgit.h"
@@ -57,9 +56,8 @@ static struct strbuf *capture;
 
 static void write_out(const char *data, size_t size)
 {
-	// A blob, a snapshot or a patch reaches this with a size well past what
-	// one write can move onto a pipe, so a short write is ordinary rather
-	// than an error and has to be resumed instead of reported.
+	// A blob or snapshot is well past what one write can move onto a
+	// pipe, so short writes are resumed rather than reported.
 	if (write_in_full(STDOUT_FILENO, data, size) < 0)
 		die_errno("write error on html output");
 }
@@ -256,11 +254,11 @@ void html_url_path(const char *txt)
 	const char *p = txt;
 	while (p && *p) {
 		unsigned char c = *p;
-		// A raw ampersand or plus is legal in a URL path, but the
-		// paths written here land in attribute values, where a bare
-		// ampersand can start a character reference and quietly turn
-		// "a&copy.txt" into a different filename. Encoding both keeps
-		// the output byte-safe in every sink.
+		// A raw ampersand or plus is legal in a URL path, but this
+		// table is shared with html_url_arg, where a bare plus decodes
+		// back as a space, and the paths written here land in attribute
+		// values, where an ampersand can start a character reference
+		// and quietly turn "a&copy.txt" into a different filename.
 		const char *esc = url_escape_table[c];
 		if (esc) {
 			html_raw(txt, p - txt);
diff --git a/source/html.h b/source/html.h
index ecc7141..3bbb30e 100644
--- a/source/html.h
+++ b/source/html.h
@@ -14,8 +14,8 @@
 
 // How much of a generated run to build before handing it to html_raw. Growing
 // past this gains nothing, since html_raw gathers what it is given into a
-// buffer of its own, and a run built whole would instead be sized by the file
-// it came from, which the blob limits do not bound.
+// buffer of its own, and a run built whole would grow with the file it came
+// from, to many times the blob's size, which max-blob-size never measures.
 #define HTML_BATCH (64 * 1024)
 
 extern void html_raw(const char *txt, size_t size);
diff --git a/source/parsing.c b/source/parsing.c
index de2798a..745bbba 100644
--- a/source/parsing.c
+++ b/source/parsing.c
@@ -77,9 +77,6 @@ static const char *reencode(char **text, const char *from, const char *to)
 {
 	char *converted;
 
-	if (!text)
-		return NULL;
-
 	if (!*text || !from || !to)
 		return *text;
 
@@ -160,7 +157,7 @@ struct commitinfo *cgit_parse_commit(struct commit *commit)
 	while (skip_prefix(p, "parent ", &p))
 		p += the_hash_algo->hexsz + 1;
 
-	if (p && skip_prefix(p, "author ", &p)) {
+	if (skip_prefix(p, "author ", &p)) {
 		parse_user(p, &info->author, &info->author_email,
 			&info->author_date, &info->author_tz);
 		p = next_header_line(p);
@@ -197,10 +194,8 @@ struct commitinfo *cgit_parse_commit(struct commit *commit)
 			eol++;
 		info->msg = xstrdup(eol);
 	} else {
-		// Reached when an object is truncated mid header, which
-		// leaves nothing at all after them. Callers render subject
-		// and msg as text without checking, so they get empty
-		// strings rather than NULL.
+		// An object truncated mid header leaves nothing after it,
+		// and callers render subject and msg without NULL checks.
 		info->subject = xstrdup("");
 		info->msg = xstrdup("");
 	}
diff --git a/source/parsing.h b/source/parsing.h
index 2230eaa..b1dc9f1 100644
--- a/source/parsing.h
+++ b/source/parsing.h
@@ -10,8 +10,8 @@
 
 #include "cgit.h"
 
-// Split PATH_INFO into the repository and the page it names, storing both in
-// ctx.qry.
+// Split a request url into the repository, the page and the path it names,
+// storing them in ctx.qry.
 extern void cgit_parse_url(const char *url);
 
 extern struct commitinfo *cgit_parse_commit(struct commit *commit);
diff --git a/source/scan-tree.c b/source/scan-tree.c
index ac74eb3..f72a5be 100644
--- a/source/scan-tree.c
+++ b/source/scan-tree.c
@@ -15,7 +15,9 @@
 
 // Git writes this line into the description file of every repository it
 // creates, so a repository still carrying it gets cgit's own default instead.
-static const char *default_git_desc = "Unnamed repository; edit this file 'description' to name the repository.";
+static const char *default_git_desc =
+	"Unnamed repository; edit this file 'description' to name the "
+	"repository.";
 
 // The config callbacks are handed only a name and a value, so the repository
 // being filled in waits here for the length of one add_repo call.
@@ -88,7 +90,7 @@ static char *section_slash(struct strbuf *relpath, int depth)
 
 	if (depth > 0) {
 		slash = relpath->buf - 1;
-		while (slash && depth && (slash = strchr(slash + 1, '/')))
+		while (depth && (slash = strchr(slash + 1, '/')))
 			depth--;
 	} else {
 		slash = relpath->buf + relpath->len;
@@ -150,9 +152,8 @@ static void add_repo(const char *base, struct strbuf *path)
 	else
 		strbuf_addstr(&relpath, path->buf + strlen(base) + 1);
 
-	// Drop the trailing slash added above before testing for "/.git", since
-	// with it still attached the suffix never matches and every ordinary
-	// working tree is named "repo/.git" instead of "repo".
+	// Drop the trailing slash added above, or the "/.git" suffix test
+	// below never matches and a working tree is named "repo/.git".
 	if (relpath.len && relpath.buf[relpath.len - 1] == '/')
 		strbuf_setlen(&relpath, relpath.len - 1);
 	if (relpath.len >= 5 && !strcmp(relpath.buf + relpath.len - 5, "/.git"))
diff --git a/source/shared.c b/source/shared.c
index 00d5605..fab5c13 100644
--- a/source/shared.c
+++ b/source/shared.c
@@ -89,9 +89,8 @@ static int load_mmfile(mmfile_t *file, const struct object_id *oid)
 }
 
 /*
- * The test is on the oid rather than on the size, because load_mmfile uses a
- * literal only for a null oid, and a real blob that happens to be empty does
- * own its buffer.
+ * The test is on the oid rather than the size, because load_mmfile uses a
+ * literal only for a null oid, while a real but empty blob owns its buffer.
  */
 static void release_mmfile(mmfile_t *file, const struct object_id *oid)
 {
@@ -101,9 +100,7 @@ static void release_mmfile(mmfile_t *file, const struct object_id *oid)
 
 /*
  * Xdiff emits buffers that need not end on a line boundary, so a trailing
- * fragment is held back and joined with whatever arrives next. Git's own
- * xdiff_outf keeps that fragment in a callback struct, which is not an option
- * here because priv already carries the caller's function.
+ * fragment is held back and joined with whatever arrives next.
  */
 static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
 {
@@ -138,9 +135,8 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
 	return 0;
 }
 
-// Takes an unsigned char because a byte over 0x7f is negative in a plain
-// char wherever char is signed, and a negative one is not a value the ctype
-// tests are defined for.
+// Takes an unsigned char because a byte over 0x7f is negative where char
+// is signed, which the ctype tests are not defined for.
 static int is_token_char(unsigned char c)
 {
 	return isalnum(c) || c == '_';
@@ -466,9 +462,8 @@ void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char *prefix)
 }
 
 /*
- * Git's parse_date_format dies on anything it does not recognize, which would
- * turn a typo in cgitrc into a failed request, so only the formats cgit
- * documents reach it.
+ * Git's parse_date_format dies on anything unknown, which would turn a
+ * cgitrc typo into a failed request, so only documented formats reach it.
  */
 void cgit_parse_date_format(const char *format, struct date_mode *mode)
 {
@@ -578,9 +573,8 @@ char *cgit_expand_macros(const char *text)
 					out = expand_macro(start, limit - start) - 1;
 				}
 				start = NULL;
-				// Step back so the character that ended the
-				// token is written again past the expansion,
-				// where it may open a token of its own.
+				// Step back so the byte that ended the token
+				// is written again and may open a new one.
 				text--;
 			}
 			out++;
diff --git a/source/ui-atom.c b/source/ui-atom.c
index 09d3501..3eb4332 100644
--- a/source/ui-atom.c
+++ b/source/ui-atom.c
@@ -28,9 +28,9 @@ static const char *feed_date(timestamp_t when)
 #define XML_REPLACEMENT "?"
 
 /*
- * How many bytes the UTF-8 sequence at p holds, or 0 when the bytes there are
- * not a valid sequence. The lead-byte ranges fold in the overlong, surrogate
- * and out-of-range cases, so a 0 is the only error signal a caller needs.
+ * How many bytes the UTF-8 sequence at p holds, or 0 when invalid. The
+ * lead-byte ranges fold in the overlong, surrogate and out-of-range cases,
+ * so 0 is the only error signal a caller needs.
  */
 static size_t utf8_seq_len(const unsigned char *p, size_t left)
 {
@@ -61,10 +61,9 @@ static size_t utf8_seq_len(const unsigned char *p, size_t left)
 }
 
 /*
- * The XML counterpart of html_txt. Commit metadata is arbitrary bytes, and
- * where a browser shrugs at a stray control byte or a broken UTF-8 sequence,
- * an XML reader must reject the whole feed, so both are replaced instead of
- * passed through.
+ * The XML counterpart of html_txt. A browser shrugs at a stray control byte
+ * or broken UTF-8, but an XML reader must reject the whole feed, so both
+ * are replaced instead of passed through.
  */
 static void xml_txt(const char *txt)
 {
@@ -219,8 +218,7 @@ void cgit_print_atom(char *tip, const char *path, int max_count)
 	prepare_revision_walk(&rev);
 
 	// CGI guarantees a server name, so only a bare test run reaches the
-	// fallback, and a placeholder there keeps the mandatory feed id and
-	// the links present rather than dropping them.
+	// fallback, which keeps the mandatory feed id and links present.
 	host = cgit_hosturl();
 	if (!host)
 		host = xstrdup("localhost");
@@ -276,9 +274,8 @@ void cgit_print_atom(char *tip, const char *path, int max_count)
 		commit->parents = NULL;
 	}
 	if (need_updated) {
-		// Atom makes a feed level updated mandatory, and an empty feed
-		// has no commit to take one from, so the epoch stands in and
-		// keeps the feed byte for byte stable.
+		// Atom makes a feed level updated mandatory, and an empty
+		// feed has no commit to take one from, so the epoch stands in.
 		html("<updated>");
 		xml_txt(feed_date(0));
 		html("</updated>\n");
diff --git a/source/ui-blame.c b/source/ui-blame.c
index ff3795e..8a7f8f0 100644
--- a/source/ui-blame.c
+++ b/source/ui-blame.c
@@ -17,9 +17,8 @@
 #include "ui-blame.h"
 #include "ui-shared.h"
 
-// A tab in the rendered source runs on to the next multiple of this. The
-// stylesheet leaves tab-size alone, so the measurement has to match what the
-// browser does on its own rather than anything cgit picks.
+// A tab in the rendered source runs on to the next multiple of this,
+// matching what the browser does on its own since tab-size is left alone.
 #define TAB_WIDTH 8
 
 enum blame_target {
@@ -225,9 +224,9 @@ static size_t line_width(struct blame_scoreboard *sb, int line)
 }
 
 /*
- * The stylesheet takes the source pre out of flow and positions it over these
- * blocks, so nothing else gives the cell a size and each block has to be
- * padded to the height and the width of the lines it stands behind.
+ * The stylesheet takes the source pre out of flow and positions it over
+ * these blocks, so each block has to be padded to the height and width of
+ * the lines it stands behind.
  */
 static void emit_entry_background(struct blame_scoreboard *sb,
 				  struct blame_entry *ent)
diff --git a/source/ui-blob.c b/source/ui-blob.c
index efe1596..67af675 100644
--- a/source/ui-blob.c
+++ b/source/ui-blob.c
@@ -72,9 +72,8 @@ static int find_path_oid(struct object_id *oid, char *path, int file_only)
 }
 
 /*
- * Callers ask before reading the object, because the point of the limit is to
- * keep a huge blob out of memory rather than to notice it once it is already
- * there.
+ * Callers ask before reading the object, so a huge blob stays out of
+ * memory rather than being noticed once already there.
  */
 static int over_size_limit(unsigned long size)
 {
diff --git a/source/ui-clone.c b/source/ui-clone.c
index 75e5f95..f5c4d59 100644
--- a/source/ui-clone.c
+++ b/source/ui-clone.c
@@ -5,7 +5,6 @@
  * files under objects, and HEAD, each written as raw bytes rather than as a
  * page. The two listings a clone starts from are built per request, so a
  * repository serves without anyone having run git update-server-info over it.
- * The shape of it all follows git's own http-backend.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
diff --git a/source/ui-diff.c b/source/ui-diff.c
index e08b096..3c7d8d2 100644
--- a/source/ui-diff.c
+++ b/source/ui-diff.c
@@ -17,12 +17,10 @@
 #include "ui-shared.h"
 #include "ui-ssdiff.h"
 
-// A file's body is collected while the walk still has that file open, because
-// the stat table above it has to be printed first and rendering the bodies
-// afterwards meant a second walk with its own rename detection and its own
-// xdiff of every file. This bounds what one request may hold that way, so it
-// is not something to configure, and once it is passed the collected bodies
-// are dropped and that second walk happens after all.
+// A file's body is collected while the walk still has that file open,
+// since the stat table above it has to be printed first. This bounds what
+// one request may hold that way. Past it the collected bodies are dropped
+// and a second walk renders the page instead.
 #define BODY_BUDGET (8 * 1024 * 1024)
 
 // What a context of zero means once the diff runs, mirroring the fallback in
@@ -81,9 +79,8 @@ static void release_bodies(void)
 
 /*
  * One bar segment of the per-file diffstat graph. The bar is a fixed-layout
- * table whose row always spans 1000 columns, so a segment takes its share
- * of the width through colspan rather than through an inline style, which
- * a Content-Security-Policy would have to allow.
+ * table whose row always spans 1000 columns, so a segment takes its width
+ * through colspan rather than an inline style a CSP would have to allow.
  */
 static void print_graph_cell(const char *class, int span)
 {
@@ -168,9 +165,8 @@ static void print_fileinfo(struct fileinfo *info)
 }
 
 /*
- * Counting is only half of what this does. It also renders each line, until
- * max-diff-lines is passed and the rest of the file is dropped, which is why
- * the two cannot be separated into a counting pass and a rendering one.
+ * Counting is only half of what this does. It also renders each line until
+ * max-diff-lines is passed, so the two cannot be split into separate passes.
  */
 static void count_diff_lines(char *line, int len)
 {
@@ -376,9 +372,9 @@ static void collect_body(struct diff_filepair *pair, struct fileinfo *item,
 	render_suppressed = 0;
 
 	if (S_ISGITLINK(pair->one->mode) || S_ISGITLINK(pair->two->mode)) {
-		// The stat has always counted what a diff of the pair produces
-		// rather than the two lines the body shows, so run that diff
-		// for the count alone.
+		// The stat counts what a diff of the pair produces rather
+		// than the two lines the body shows, so run that diff for
+		// the count alone.
 		render_line_fn = NULL;
 		cgit_diff_files(&pair->one->oid, &pair->two->oid, old_size,
 				new_size, binary, 0, ctx.qry.ignorews,
@@ -398,10 +394,9 @@ static void collect_body(struct diff_filepair *pair, struct fileinfo *item,
 		cgit_ssdiff_footer();
 
 	if (render_suppressed) {
-		// Setting the length back would leave the buffer holding
-		// everything it grew to while rendering, which the budget below
-		// cannot see because it only counts what is kept, so rebuild it
-		// at the size actually kept.
+		// Setting the length back would keep the grown allocation,
+		// which the budget below cannot see because it only counts
+		// what is kept, so rebuild the buffer at the kept size.
 		char *header_text = xmemdupz(body->buf, header_len);
 
 		strbuf_release(body);
diff --git a/source/ui-log.c b/source/ui-log.c
index bbfeb67..5b5dd32 100644
--- a/source/ui-log.c
+++ b/source/ui-log.c
@@ -337,7 +337,7 @@ static char *next_token(char **src)
 {
 	char *token;
 
-	if (!src || !*src)
+	if (!*src)
 		return NULL;
 	while (isspace((unsigned char)**src))
 		(*src)++;
@@ -398,7 +398,6 @@ 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;
@@ -463,10 +462,9 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep,
 	tip = disambiguate_ref(tip, &must_free_tip);
 	if (tip && tip[0] == '-') {
 		// 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.
+		// option, so a tip like "--output=<path>" would become 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);
@@ -484,10 +482,10 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep,
 			char *arg;
 
 			// 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.
+			// revision expression only, since a leading dash
+			// would reach setup_revisions as a rev-list option.
+			// The tip pushed above goes away, since the range
+			// supersedes it.
 			strvec_pop(&rev_argv);
 			while ((arg = next_token(&pattern))) {
 				if (*arg == '-') {
diff --git a/source/ui-patch.c b/source/ui-patch.c
index ef63d00..bc98f2e 100644
--- a/source/ui-patch.c
+++ b/source/ui-patch.c
@@ -1,11 +1,9 @@
 /*
  * The patch page, which serves a commit or a range of commits as plain text
  * in the mail format git format-patch writes, so that a change read in cgit
- * can be fed straight to git am. It is one of the repository commands in
- * cmd.c, taking the newer revision from id, the older one from id2, and an
- * optional path that narrows the diff. A merge carries no single patch and so
- * drops out of a range, and max-patch-count bounds how many commits one
- * request may emit.
+ * can be fed straight to git am. A merge carries no single patch and drops
+ * out of a range, and max-patch-count bounds how many commits one request
+ * may emit.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
diff --git a/source/ui-plain.c b/source/ui-plain.c
index b63b9c6..d3b4771 100644
--- a/source/ui-plain.c
+++ b/source/ui-plain.c
@@ -45,10 +45,10 @@ static int is_unsafe_type(const char *mimetype)
 }
 
 /*
- * A nonzero return says the response has been written, error pages included,
- * so the walk does not go on to report the path as missing.
+ * Writes the response for the object, error pages included, so the walk
+ * does not go on to report the path as missing.
  */
-static int print_object(const struct object_id *oid, const char *path)
+static void print_object(const struct object_id *oid, const char *path)
 {
 	enum object_type type;
 	char *buf, *mimetype;
@@ -57,7 +57,7 @@ static int print_object(const struct object_id *oid, const char *path)
 	type = odb_read_object_info(the_repository->objects, oid, &size);
 	if (type == OBJ_BAD) {
 		cgit_print_error_page(404, "Not Found", "Not found");
-		return 1;
+		return;
 	}
 
 	// The limit counts kilobytes and is checked before the read, so a huge
@@ -67,13 +67,13 @@ static int print_object(const struct object_id *oid, const char *path)
 		cgit_print_error_page(413, "Content Too Large",
 			"Object size (%luKB) exceeds limit (%dKB)",
 			size / 1024, ctx.cfg.max_blob_size);
-		return 1;
+		return;
 	}
 
 	buf = odb_read_object(the_repository->objects, oid, &type, &size);
 	if (!buf) {
 		cgit_print_error_page(404, "Not Found", "Not found");
-		return 1;
+		return;
 	}
 
 	mimetype = cgit_get_mimetype_for_filename(path);
@@ -99,7 +99,6 @@ static int print_object(const struct object_id *oid, const char *path)
 	html_raw(buf, size);
 	free(mimetype);
 	free(buf);
-	return 1;
 }
 
 static char *build_path(const char *base, int baselen, const char *path)
@@ -110,8 +109,7 @@ static char *build_path(const char *base, int baselen, const char *path)
 		return cgit_fmtalloc("%.*s/", baselen, base);
 }
 
-static void print_dir(const struct object_id *oid, const char *base,
-		      int baselen, const char *path)
+static void print_dir(const char *base, int baselen, const char *path)
 {
 	char *fullpath;
 	const char *leading_slash;
@@ -120,9 +118,8 @@ static void print_dir(const struct object_id *oid, const char *base,
 	fullpath = build_path(base, baselen, path);
 	leading_slash = (fullpath[0] == '/' ? "" : "/");
 	cgit_print_http_headers();
-	// The listing is a full document of its own, so it carries the same
-	// doctype and charset as the layout pages or the browser would parse
-	// it in quirks mode.
+	// A full document of its own, and without the doctype and charset
+	// the browser would parse it in quirks mode.
 	html("<!DOCTYPE html>\n<html lang='en'>\n<head>\n");
 	html("<meta charset='UTF-8'>\n");
 	htmlf("<title>%s", leading_slash);
@@ -186,10 +183,10 @@ static int walk_tree(const struct object_id *oid, struct strbuf *base,
 
 	if (walk->dir_len >= 0 && base->len == (size_t)walk->dir_len) {
 		if (S_ISREG(mode) || S_ISLNK(mode)) {
-			if (print_object(oid, pathname))
-				walk->response = RESPONSE_BLOB;
+			print_object(oid, pathname);
+			walk->response = RESPONSE_BLOB;
 		} else if (S_ISDIR(mode)) {
-			print_dir(oid, base->buf, base->len, pathname);
+			print_dir(base->buf, base->len, pathname);
 			walk->response = RESPONSE_LISTING;
 			return READ_TREE_RECURSIVE;
 		}
@@ -217,10 +214,9 @@ void cgit_print_plain(void)
 	struct object_id oid;
 	struct commit *commit;
 	int path_len = ctx.qry.path ? strlen(ctx.qry.path) : 0;
-	// A hand built pathspec leaves nowildcard_len at zero, which tells git
-	// the match may be a glob. It would then hand this walk every entry a
-	// pattern like * matches, and each one would be answered with its own
-	// set of HTTP headers inside the body of the first.
+	// nowildcard_len matches len so git treats the path as literal. As a
+	// glob, every entry a pattern like * matches would be answered with
+	// its own HTTP headers inside the body of the first.
 	struct pathspec_item path_items = {
 		.match = ctx.qry.path,
 		.len = path_len,
@@ -251,7 +247,7 @@ void cgit_print_plain(void)
 		// itself, so the listing it would have opened is opened here.
 		path_items.match = "";
 		walk.dir_len = -1;
-		print_dir(get_commit_tree_oid(commit), "", 0, "");
+		print_dir("", 0, "");
 		walk.response = RESPONSE_LISTING;
 	} else {
 		walk.dir_len = dir_prefix_len(path_items.match);
diff --git a/source/ui-repolist.c b/source/ui-repolist.c
index 39978ae..9f60228 100644
--- a/source/ui-repolist.c
+++ b/source/ui-repolist.c
@@ -267,7 +267,7 @@ static void print_repo_row(const char *currenturl, int sublevel)
 static void print_pager(int total, int pagelen, char *search, char *sort)
 {
 	int i, ofs;
-	char *class = NULL;
+	char *class;
 
 	html("<ul class='pager'>\n");
 	for (i = 0, ofs = 0; ofs < total; i++, ofs = i * pagelen) {
@@ -356,8 +356,7 @@ static int cmp_section(const void *a, const void *b)
 
 /*
  * get_repo_modtime caches into the repository it is handed, but qsort moves
- * those structs around as it works, so a comparator left to fill the cache
- * loses most of what it stored and stats the same repository over and over.
+ * those structs as it works, so the cache is filled before sorting.
  */
 static void resolve_modtimes(void)
 {
diff --git a/source/ui-shared.c b/source/ui-shared.c
index a79c291..2ec9d0e 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -611,7 +611,8 @@ static void print_repo_tabs(void)
 				ctx.qry.oid, ctx.qry.vpath);
 	else
 		cgit_tree_link("tree",
-			       tab_title("Browse the tree at %s", "Browse the file tree", vpath),
+			       tab_title("Browse the tree at %s",
+					 "Browse the file tree", vpath),
 			       tab_class("tree"), ctx.qry.head,
 			       ctx.qry.oid, ctx.qry.vpath);
 	html("</li>\n<li>");
@@ -686,8 +687,6 @@ static void print_site_search(void)
 	free(currenturl);
 }
 
-// The link is built out of the request in ctx.qry, so a caller that alters a
-// field of ctx.qry first gets a link differing in exactly that.
 static void snapshot_link(const char *name, const char *title, const char *class,
 			const char *head, const char *rev,
 			const char *archivename)
@@ -695,6 +694,8 @@ static void snapshot_link(const char *name, const char *title, const char *class
 	reporevlink("snapshot", name, title, class, head, rev, archivename);
 }
 
+// The link is built out of the request in ctx.qry, so a caller that alters a
+// field of ctx.qry first gets a link differing in exactly that.
 static void self_link(const char *name, const char *title, const char *class)
 {
 	if (!strcmp(ctx.qry.page, "repolist"))
@@ -1225,10 +1226,7 @@ static const struct forge *forge_for_host(const char *host, size_t len)
  * Derives a submodule row's links from its .gitmodules entry. The url is
  * matched against this instance's own repositories first, so ssh, file and
  * relative urls still land on an internal page when their target is served
- * here, and the pinned commit gets a page of its own. A plain web url is
- * linked as it is, an ssh url to a known host is rewritten to its web form,
- * and anything else is left unlinked with the url as a tooltip, since a
- * scheme cgit cannot vouch for has no place in an href.
+ * here, and a scheme cgit cannot vouch for is left unlinked as a tooltip.
  */
 static void gitmodules_link(const char *path, const char *rev,
 			    char **module, char **commit,
@@ -1477,7 +1475,8 @@ void cgit_print_http_headers(void)
 
 void cgit_redirect(const char *url, bool permanent)
 {
-	htmlf("Status: %d %s\n", permanent ? 301 : 302, permanent ? "Moved Permanently" : "Found");
+	htmlf("Status: %d %s\n", permanent ? 301 : 302,
+	      permanent ? "Moved Permanently" : "Found");
 	html("Location: ");
 	html_url_path(url);
 	html("\n\n");
@@ -1500,8 +1499,10 @@ void cgit_print_docstart(void)
 	html("<meta charset='UTF-8'>\n");
 	html("<meta name='viewport' content='width=device-width, initial-scale=1'>\n");
 	html("<meta name='color-scheme' content='light dark'>\n");
-	html("<meta name='theme-color' media='(prefers-color-scheme: light)' content='#ffffff'>\n");
-	html("<meta name='theme-color' media='(prefers-color-scheme: dark)' content='#1b1b1b'>\n");
+	html("<meta name='theme-color' media='(prefers-color-scheme: light)'"
+	     " content='#ffffff'>\n");
+	html("<meta name='theme-color' media='(prefers-color-scheme: dark)'"
+	     " content='#1b1b1b'>\n");
 	html("<title>");
 	// An error page reached before a title was chosen still has to name
 	// itself, since an empty title element is not valid.
@@ -1585,7 +1586,8 @@ void cgit_print_docend(void)
 	if (ctx.cfg.footer)
 		html_include(ctx.cfg.footer);
 	else {
-		htmlf("<footer class='footer'>generated by <a href='https://github.com/brycekwon/cgit'>cgit %s</a> "
+		htmlf("<footer class='footer'>generated by "
+			"<a href='https://github.com/brycekwon/cgit'>cgit %s</a> "
 			"(<a href='https://git-scm.com/'>git %s</a>)</footer>\n",
 			cgit_version, git_version_string);
 	}
@@ -1652,11 +1654,10 @@ static void capture_http_clone_url(const char *url)
 }
 
 /*
- * One row of the clone table. The url is shown as written, and every row is
- * a link, but a browser cannot follow the ssh, scp or git forms, so those
- * rows point at the first http url in the clone list, or at the repository's
- * own page when the list has none, which with http clone enabled is itself a
- * working clone url. A colspan of zero leaves the attribute out.
+ * One row of the clone table. Every row is a link, but a browser cannot
+ * follow the ssh, scp or git forms, so those rows point at the first http
+ * url in the clone list, or at the repository's own page when the list has
+ * none. A colspan of zero leaves the attribute out.
  */
 void cgit_print_clone_row(const char *url, int colspan)
 {
@@ -1858,7 +1859,8 @@ void cgit_print_snapshot_links(const struct cgit_repo *repo, const char *ref,
 			snapshot_link("sig", NULL, NULL, NULL, NULL,
 					   filename.buf);
 			html(")");
-		} else if (starts_with(f->suffix, ".tar") && cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[0])) {
+		} else if (starts_with(f->suffix, ".tar") &&
+			   cgit_snapshot_get_sig(ref, &cgit_snapshot_formats[0])) {
 			// A compressed tarball offers the signature made for
 			// the plain tar it expands to, which is the first
 			// format in the table.
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.c
index 7da7252..af985f4 100644
--- a/source/ui-snapshot.c
+++ b/source/ui-snapshot.c
@@ -165,7 +165,7 @@ static const char *ref_from_filename(const struct cgit_repo *repo,
 	if (starts_with(rev.buf, repo_prefix)) {
 		const char *rest = rev.buf + strlen(repo_prefix);
 
-		while (rest && (*rest == '-' || *rest == '_'))
+		while (*rest == '-' || *rest == '_')
 			rest++;
 		strbuf_splice(&rev, 0, rest - rev.buf, "", 0);
 	}
diff --git a/source/ui-snapshot.h b/source/ui-snapshot.h
index 9f1ec02..9d2910f 100644
--- a/source/ui-snapshot.h
+++ b/source/ui-snapshot.h
@@ -26,8 +26,9 @@ extern unsigned cgit_snapshot_format_bit(const struct cgit_snapshot_format *f);
 
 /*
  * Turn a cgitrc snapshots value into a mask over cgit_snapshot_formats. A
- * plain number is the legacy form meaning every format, as is the word all,
- * otherwise the value is a space separated list of suffixes.
+ * plain nonzero number is the legacy boolean form and enables plain tar
+ * alone, the word all enables every format, and otherwise the value is a
+ * space separated list of suffixes.
  */
 extern int cgit_parse_snapshots_mask(const char *str);
 
diff --git a/source/ui-ssdiff.c b/source/ui-ssdiff.c
index 8651ce7..759a77b 100644
--- a/source/ui-ssdiff.c
+++ b/source/ui-ssdiff.c
@@ -33,8 +33,7 @@ static struct deferred_line *deferred_new, *deferred_new_last;
 /*
  * The table is reused by every comparison and nothing clears it in between,
  * because the fill in longest_common_subsequence works back from the far
- * corner and writes every cell it goes on to read. Clearing it for each pair
- * of lines cost more than the comparison it was preparing for.
+ * corner and writes every cell it goes on to read.
  */
 static void create_lcs_table(void)
 {
@@ -107,9 +106,8 @@ static char *longest_common_subsequence(const char *old_line,
 }
 
 /*
- * The line with its tabs expanded, which the caller owns. Appending as the
- * line is walked replaces a loop that rescanned the rest of the input at every
- * tab, which made a tab heavy line quadratic in its own length.
+ * The line with its tabs expanded, which the caller owns. Built in one
+ * forward pass so a tab heavy line stays linear in its own length.
  */
 static char *expand_tabs(const char *line)
 {
@@ -135,9 +133,8 @@ static void flush_run(struct strbuf *run)
 }
 
 /*
- * A stretch that the other side does not share is escaped in one call because
- * escaping a character at a time sent every character of every changed line
- * through the output path on its own, which dominated this page.
+ * A stretch the other side does not share is escaped in one call, so a
+ * changed line does not go through the output path a byte at a time.
  */
 static void print_line_with_lcs(const char *class, const char *line,
 				const char *lcs)
@@ -397,8 +394,7 @@ void cgit_ssdiff_line_cb(char *line, int len)
 	}
 
 	if (line[0] == ' ') {
-		if (deferred_old || deferred_new)
-			print_deferred_lines();
+		print_deferred_lines();
 		print_row("ctx", current_old_line, line,
 			  current_new_line, line, 0);
 		current_old_line += 1;
@@ -438,7 +434,6 @@ void cgit_ssdiff_header_end(void)
 
 void cgit_ssdiff_footer(void)
 {
-	if (deferred_old || deferred_new)
-		print_deferred_lines();
+	print_deferred_lines();
 	html("<tr><td class='foot' colspan='4'></td></tr>\n");
 }
diff --git a/source/ui-stats.c b/source/ui-stats.c
index 5b36f54..62aac56 100644
--- a/source/ui-stats.c
+++ b/source/ui-stats.c
@@ -149,10 +149,10 @@ static char *pretty_year(struct tm *tm)
  * caps the page by storing an index into this table as its max-stats.
  */
 static const struct cgit_period periods[] = {
-	{'w', "week", 12, 4, trunc_week, dec_week, inc_week, pretty_week},
-	{'m', "month", 12, 4, trunc_month, dec_month, inc_month, pretty_month},
-	{'q', "quarter", 12, 4, trunc_quarter, dec_quarter, inc_quarter, pretty_quarter},
-	{'y', "year", 12, 4, trunc_year, dec_year, inc_year, pretty_year},
+	{'w', "week", 4, trunc_week, dec_week, inc_week, pretty_week},
+	{'m', "month", 4, trunc_month, dec_month, inc_month, pretty_month},
+	{'q', "quarter", 4, trunc_quarter, dec_quarter, inc_quarter, pretty_quarter},
+	{'y', "year", 4, trunc_year, dec_year, inc_year, pretty_year},
 };
 
 static void window_start(const struct cgit_period *period, struct tm *tm)
diff --git a/source/ui-stats.h b/source/ui-stats.h
index ccb0598..1b20e6c 100644
--- a/source/ui-stats.h
+++ b/source/ui-stats.h
@@ -13,7 +13,6 @@
 struct cgit_period {
 	const char code;
 	const char *name;
-	int max_periods;
 
 	// How many periods a page shows side by side.
 	int count;
diff --git a/source/ui-summary.c b/source/ui-summary.c
index dacf0f7..197cd07 100644
--- a/source/ui-summary.c
+++ b/source/ui-summary.c
@@ -65,10 +65,9 @@ static int path_within(const char *base, const char *path)
 
 /*
  * Returns a path the caller must free, or NULL when the request cannot be
- * served. A null ref means the readme is a file on the server's disk rather
- * than a path inside a ref, and such a readme is confined to its own
- * directory, so one named without a directory has nothing to confine it to
- * and is refused.
+ * served. A null ref means the readme is a file on the server's disk, and
+ * such a readme is confined to its own directory, so one named without a
+ * directory is refused.
  */
 static char *resolve_about_path(const char *filename, const char *ref,
 				const char *path)
@@ -158,10 +157,8 @@ void cgit_print_repo_readme(const char *path)
 
 	html("<div id='summary'>");
 	if (!ctx.repo->about_filter) {
-		// With no about-filter configured there is nothing to turn the
-		// readme source into safe HTML, so it is escaped rather than
-		// served raw, which would let an untrusted repository put
-		// script on this page.
+		// With no about-filter there is nothing to turn the readme
+		// into safe HTML, so it is escaped rather than served raw.
 		html("<pre class='plaintext'>");
 		if (ref) {
 			cgit_print_file(filename, ref, 1, 1);
diff --git a/source/ui-tree.c b/source/ui-tree.c
index cbf783f..26a3fd1 100644
--- a/source/ui-tree.c
+++ b/source/ui-tree.c
@@ -53,10 +53,8 @@ struct only_child {
 };
 
 /*
- * A formatted write per line meant a syscall and a temporary buffer for every
- * line of the file, so the anchors are handed over in batches. Building the
- * column whole was rejected because it would come to several times the size of
- * the blob.
+ * The anchors are handed over in batches rather than a write per line, and
+ * never built whole, which would come to several times the blob's size.
  */
 static void print_linenumbers(const char *buf, unsigned long size)
 {
@@ -109,8 +107,6 @@ static void print_text_buffer(const char *filename, char *buf,
 		return;
 	}
 
-	// Syntax highlighting ships as one of the filters under
-	// custom/extensions, which keeps knowledge of languages out of cgit.
 	html("<td class='lines'><pre><code>");
 	html_txt(buf);
 	html("</code></pre></td></tr></table>\n");
@@ -134,8 +130,7 @@ static void print_binary_buffer(char *buf, unsigned long size)
 	html("<table class='bin-blob'>\n");
 	html("<tr><th>ofs</th><th>hex dump</th><th>ascii</th></tr>\n");
 
-	// At the default blob size limit a write per byte spent almost all of
-	// its time in the kernel, so a row goes out in one write.
+	// A row goes out in one write rather than a write per byte.
 	for (offset = 0; offset < size;
 	     offset += HEXDUMP_ROW_BYTES, buf += HEXDUMP_ROW_BYTES) {
 		strbuf_reset(&row);
@@ -424,7 +419,7 @@ static void ls_tail(void)
 	cgit_print_layout_end();
 }
 
-static void ls_tree(const struct object_id *oid, const char *path,
+static void ls_tree(const struct object_id *oid,
 		    struct walk_tree_context *walk)
 {
 	struct tree *tree;
@@ -518,7 +513,7 @@ void cgit_print_tree(const char *rev, char *path)
 	walk.rev = xstrdup(rev);
 
 	if (path == NULL) {
-		ls_tree(get_commit_tree_oid(commit), NULL, &walk);
+		ls_tree(get_commit_tree_oid(commit), &walk);
 		goto cleanup;
 	}