Restyle the sources and fix the audit's findings

1 file changed, 302 insertions, 316 deletions

diff --git a/source/shared.c b/source/shared.c
index 259802a..e6dd011 100644
--- a/source/shared.c
+++ b/source/shared.c
@@ -1,24 +1,172 @@
-/* shared.c: global vars + some callback functions
- *
- * 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 helpers that no single page owns, along with the process wide state
+ * every page reads. Repositories are registered here as cgitrc is parsed, the
+ * reference and commit records that the renderers list are built and freed
+ * here, and git's diff machinery is wrapped so a caller is handed whole
+ * filepairs or whole lines instead of xdiff buffers.
  */
 
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "cgit.h"
+#include "parsing.h"
+#include "shared.h"
 
-/* Longest result cgit_expand_macros can return. */
 #define MACRO_EXPANSION_BUFSIZE (1024 * 8)
 
-/* Partial line carried between xdiff callbacks, see filediff_cb. */
-static char *diffbuf;
-static int diffbuf_len;
+// The number of context lines git itself defaults to.
+#define DEFAULT_DIFF_CONTEXT 3
+
+typedef struct {
+	const char *name;
+	const char *value;
+} env_var;
 
 struct cgit_repolist cgit_repolist;
 struct cgit_context ctx;
+char *cgit_default_repo_desc = "[no description]";
+
+// A diff line that arrived in pieces and is still waiting for its end, see
+// emit_line.
+static char *fragment;
+static int fragment_len;
+
+static void free_refinfo(struct refinfo *ref)
+{
+	free((char *)ref->refname);
+	switch (ref->object->type) {
+	case OBJ_TAG:
+		cgit_free_taginfo(ref->tag);
+		break;
+	case OBJ_COMMIT:
+		cgit_free_commitinfo(ref->commit);
+		break;
+	}
+	free(ref);
+}
+
+static struct refinfo *make_refinfo(const char *refname,
+				    const struct object_id *oid)
+{
+	struct refinfo *ref;
+
+	ref = xcalloc(1, sizeof(struct refinfo));
+	ref->refname = xstrdup(refname);
+	ref->object = parse_object(the_repository, oid);
+	if (!ref->object) {
+		free((char *)ref->refname);
+		free(ref);
+		return NULL;
+	}
+	switch (ref->object->type) {
+	case OBJ_TAG:
+		ref->tag = cgit_parse_tag((struct tag *)ref->object);
+		break;
+	case OBJ_COMMIT:
+		ref->commit = cgit_parse_commit((struct commit *)ref->object);
+		break;
+	}
+	return ref;
+}
+
+static int load_mmfile(mmfile_t *file, const struct object_id *oid)
+{
+	enum object_type type;
+
+	// A null oid is the absent side of an add or a delete, and diffs as an
+	// empty file rather than as a failure.
+	if (is_null_oid(oid)) {
+		file->ptr = (char *)"";
+		file->size = 0;
+		return 1;
+	}
+
+	file->ptr = odb_read_object(the_repository->objects, oid, &type,
+				    (unsigned long *)&file->size);
+	// odb_read_object leaves size untouched when it fails, so the caller
+	// has to be told rather than handed a buffer with an unset length.
+	return file->ptr != NULL;
+}
+
+/*
+ * 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.
+ */
+static void release_mmfile(mmfile_t *file, const struct object_id *oid)
+{
+	if (!is_null_oid(oid))
+		free(file->ptr);
+}
+
+/*
+ * 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.
+ */
+static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
+{
+	linediff_fn emit = (linediff_fn)priv;
+	int i;
+
+	for (i = 0; i < nbuf; i++) {
+		if (mb[i].ptr[mb[i].size-1] != '\n') {
+			fragment = xrealloc(fragment, fragment_len + mb[i].size);
+			memcpy(fragment + fragment_len, mb[i].ptr, mb[i].size);
+			fragment_len += mb[i].size;
+			continue;
+		}
+
+		if (!fragment) {
+			emit(mb[i].ptr, mb[i].size);
+			continue;
+		}
+		fragment = xrealloc(fragment, fragment_len + mb[i].size);
+		memcpy(fragment + fragment_len, mb[i].ptr, mb[i].size);
+		emit(fragment, fragment_len + mb[i].size);
+		free(fragment);
+		fragment = NULL;
+		fragment_len = 0;
+	}
+	if (fragment) {
+		emit(fragment, fragment_len);
+		free(fragment);
+		fragment = NULL;
+		fragment_len = 0;
+	}
+	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.
+static int is_token_char(unsigned char c)
+{
+	return isalnum(c) || c == '_';
+}
+
+/*
+ * Replace the variable name held at name with its value from the environment,
+ * writing at most room bytes including the terminator, and return where the
+ * text now ends.
+ */
+static char *expand_macro(char *name, size_t room)
+{
+	char *value;
+	size_t len;
+
+	len = 0;
+	value = getenv(name);
+	if (value) {
+		len = strlen(value) + 1;
+		if (len > room)
+			len = room;
+		strlcpy(name, value, len);
+		--len;
+	}
+	return name + len;
+}
 
 int cgit_die_unless_zero(int result, const char *msg)
 {
@@ -41,10 +189,9 @@ int cgit_die_unless_non_negative(int result, const char *msg)
 	return result;
 }
 
-char *cgit_default_repo_desc = "[no description]";
 struct cgit_repo *cgit_add_repo(const char *url)
 {
-	struct cgit_repo *ret;
+	struct cgit_repo *repo;
 
 	if (++cgit_repolist.count > cgit_repolist.length) {
 		if (cgit_repolist.length == 0)
@@ -56,42 +203,42 @@ struct cgit_repo *cgit_add_repo(const char *url)
 					       sizeof(struct cgit_repo));
 	}
 
-	ret = &cgit_repolist.repos[cgit_repolist.count-1];
-	memset(ret, 0, sizeof(struct cgit_repo));
-	ret->url = cgit_trim_end(url, '/');
-	if (ret->url)
-		*strchrnul(ret->url, '\n') = '\0';
-	ret->name = ret->url;
-	ret->path = NULL;
-	ret->desc = cgit_default_repo_desc;
-	ret->extra_head_content = NULL;
-	ret->owner = NULL;
-	ret->homepage = NULL;
-	ret->section = ctx.cfg.section;
-	ret->snapshots = ctx.cfg.snapshots;
-	ret->enable_blame = ctx.cfg.enable_blame;
-	ret->enable_commit_graph = ctx.cfg.enable_commit_graph;
-	ret->enable_follow_links = ctx.cfg.enable_follow_links;
-	ret->enable_log_filecount = ctx.cfg.enable_log_filecount;
-	ret->enable_log_linecount = ctx.cfg.enable_log_linecount;
-	ret->enable_remote_branches = ctx.cfg.enable_remote_branches;
-	ret->enable_subject_links = ctx.cfg.enable_subject_links;
-	ret->enable_html_serving = ctx.cfg.enable_html_serving;
-	ret->enable_stats = ctx.cfg.enable_stats;
-	ret->max_stats = ctx.cfg.max_stats;
-	ret->branch_sort = ctx.cfg.branch_sort;
-	ret->commit_sort = ctx.cfg.commit_sort;
-	ret->module_link = ctx.cfg.module_link;
-	ret->readme = ctx.cfg.readme;
-	ret->mtime = -1;
-	ret->about_filter = ctx.cfg.about_filter;
-	ret->commit_filter = ctx.cfg.commit_filter;
-	ret->source_filter = ctx.cfg.source_filter;
-	ret->email_filter = ctx.cfg.email_filter;
-	ret->clone_url = ctx.cfg.clone_url;
-	ret->submodules.strdup_strings = 1;
-	ret->hide = ret->ignore = 0;
-	return ret;
+	repo = &cgit_repolist.repos[cgit_repolist.count-1];
+	memset(repo, 0, sizeof(struct cgit_repo));
+	repo->url = cgit_trim_end(url, '/');
+	if (repo->url)
+		*strchrnul(repo->url, '\n') = '\0';
+	repo->name = repo->url;
+	repo->path = NULL;
+	repo->desc = cgit_default_repo_desc;
+	repo->extra_head_content = NULL;
+	repo->owner = NULL;
+	repo->homepage = NULL;
+	repo->section = ctx.cfg.section;
+	repo->snapshots = ctx.cfg.snapshots;
+	repo->enable_blame = ctx.cfg.enable_blame;
+	repo->enable_commit_graph = ctx.cfg.enable_commit_graph;
+	repo->enable_follow_links = ctx.cfg.enable_follow_links;
+	repo->enable_log_filecount = ctx.cfg.enable_log_filecount;
+	repo->enable_log_linecount = ctx.cfg.enable_log_linecount;
+	repo->enable_remote_branches = ctx.cfg.enable_remote_branches;
+	repo->enable_subject_links = ctx.cfg.enable_subject_links;
+	repo->enable_html_serving = ctx.cfg.enable_html_serving;
+	repo->enable_stats = ctx.cfg.enable_stats;
+	repo->max_stats = ctx.cfg.max_stats;
+	repo->branch_sort = ctx.cfg.branch_sort;
+	repo->commit_sort = ctx.cfg.commit_sort;
+	repo->module_link = ctx.cfg.module_link;
+	repo->readme = ctx.cfg.readme;
+	repo->mtime = -1;
+	repo->about_filter = ctx.cfg.about_filter;
+	repo->commit_filter = ctx.cfg.commit_filter;
+	repo->source_filter = ctx.cfg.source_filter;
+	repo->email_filter = ctx.cfg.email_filter;
+	repo->clone_url = ctx.cfg.clone_url;
+	repo->submodules.strdup_strings = 1;
+	repo->hide = repo->ignore = 0;
+	return repo;
 }
 
 struct cgit_repo *cgit_get_repoinfo(const char *url)
@@ -123,7 +270,7 @@ void cgit_free_commitinfo(struct commitinfo *info)
 
 char *cgit_trim_end(const char *str, char c)
 {
-	int len;
+	size_t len;
 
 	if (str == NULL)
 		return NULL;
@@ -156,7 +303,7 @@ void strbuf_ensure_end(struct strbuf *sb, char c)
 		strbuf_addch(sb, c);
 }
 
-void cgit_add_ref(struct reflist *list, struct refinfo *ref)
+static void add_ref(struct reflist *list, struct refinfo *ref)
 {
 	size_t size;
 
@@ -168,186 +315,95 @@ void cgit_add_ref(struct reflist *list, struct refinfo *ref)
 	list->refs[list->count++] = ref;
 }
 
-static struct refinfo *cgit_mk_refinfo(const char *refname, const struct object_id *oid)
+void cgit_free_taginfo(struct taginfo *info)
 {
-	struct refinfo *ref;
-
-	ref = xcalloc(1, sizeof (struct refinfo));
-	ref->refname = xstrdup(refname);
-	ref->object = parse_object(the_repository, oid);
-	if (!ref->object) {
-		// A dangling or unreadable ref has no object to describe.
-		free((char *)ref->refname);
-		free(ref);
-		return NULL;
-	}
-	switch (ref->object->type) {
-	case OBJ_TAG:
-		ref->tag = cgit_parse_tag((struct tag *)ref->object);
-		break;
-	case OBJ_COMMIT:
-		ref->commit = cgit_parse_commit((struct commit *)ref->object);
-		break;
-	}
-	return ref;
-}
-
-void cgit_free_taginfo(struct taginfo *tag)
-{
-	if (tag->tagger)
-		free(tag->tagger);
-	if (tag->tagger_email)
-		free(tag->tagger_email);
-	if (tag->msg)
-		free(tag->msg);
-	free(tag);
-}
-
-static void cgit_free_refinfo(struct refinfo *ref)
-{
-	if (ref->refname)
-		free((char *)ref->refname);
-	switch (ref->object->type) {
-	case OBJ_TAG:
-		cgit_free_taginfo(ref->tag);
-		break;
-	case OBJ_COMMIT:
-		cgit_free_commitinfo(ref->commit);
-		break;
-	}
-	free(ref);
+	// NULL is accepted because cgit_parse_tag returns it for a tag object
+	// that cannot be read, and a ref left holding that still gets freed
+	// along with the rest of its list.
+	if (!info)
+		return;
+	free(info->tagger);
+	free(info->tagger_email);
+	free(info->msg);
+	free(info);
 }
 
 void cgit_free_reflist_inner(struct reflist *list)
 {
 	int i;
 
-	for (i = 0; i < list->count; i++) {
-		cgit_free_refinfo(list->refs[i]);
-	}
+	for (i = 0; i < list->count; i++)
+		free_refinfo(list->refs[i]);
 	free(list->refs);
 }
 
 int cgit_refs_cb(const struct reference *ref, void *cb_data)
 {
 	struct reflist *list = (struct reflist *)cb_data;
-	struct refinfo *info = cgit_mk_refinfo(ref->name, ref->oid);
+	struct refinfo *info = make_refinfo(ref->name, ref->oid);
 
 	if (info)
-		cgit_add_ref(list, info);
+		add_ref(list, info);
 	return 0;
 }
 
 void cgit_diff_tree_cb(struct diff_queue_struct *q,
 		       struct diff_options *options, void *data)
 {
+	filepair_fn fn = (filepair_fn)data;
 	int i;
 
 	for (i = 0; i < q->nr; i++) {
+		// An unmerged path has no single pair of blobs to show.
 		if (q->queue[i]->status == 'U')
 			continue;
-		((filepair_fn)data)(q->queue[i]);
+		fn(q->queue[i]);
 	}
 }
 
-static int load_mmfile(mmfile_t *file, const struct object_id *oid)
-{
-	enum object_type type;
-
-	if (is_null_oid(oid)) {
-		file->ptr = (char *)"";
-		file->size = 0;
-	} else {
-		file->ptr = odb_read_object(the_repository->objects, oid, &type,
-		                           (unsigned long *)&file->size);
-	}
-	return 1;
-}
-
-/*
- * Receive diff-buffers from xdiff and concatenate them as
- * needed across multiple callbacks.
- *
- * This is basically a copy of xdiff-interface.c/xdiff_outf(),
- * ripped from git and modified to use globals instead of
- * a special callback-struct.
- */
-static int filediff_cb(void *priv, mmbuffer_t *mb, int nbuf)
-{
-	int i;
-
-	for (i = 0; i < nbuf; i++) {
-		if (mb[i].ptr[mb[i].size-1] != '\n') {
-			/* Incomplete line */
-			diffbuf = xrealloc(diffbuf, diffbuf_len + mb[i].size);
-			memcpy(diffbuf + diffbuf_len, mb[i].ptr, mb[i].size);
-			diffbuf_len += mb[i].size;
-			continue;
-		}
-
-		/* we have a complete line */
-		if (!diffbuf) {
-			((linediff_fn)priv)(mb[i].ptr, mb[i].size);
-			continue;
-		}
-		diffbuf = xrealloc(diffbuf, diffbuf_len + mb[i].size);
-		memcpy(diffbuf + diffbuf_len, mb[i].ptr, mb[i].size);
-		((linediff_fn)priv)(diffbuf, diffbuf_len + mb[i].size);
-		free(diffbuf);
-		diffbuf = NULL;
-		diffbuf_len = 0;
-	}
-	if (diffbuf) {
-		((linediff_fn)priv)(diffbuf, diffbuf_len);
-		free(diffbuf);
-		diffbuf = NULL;
-		diffbuf_len = 0;
-	}
-	return 0;
-}
-
 int cgit_diff_files(const struct object_id *old_oid,
 		    const struct object_id *new_oid, unsigned long *old_size,
 		    unsigned long *new_size, int *binary, int context,
 		    int ignorews, linediff_fn fn)
 {
-	mmfile_t file1, file2;
+	mmfile_t old_file, new_file;
 	xpparam_t diff_params;
 	xdemitconf_t emit_params;
 	xdemitcb_t emit_cb;
-	unsigned long size1 = 0, size2 = 0;
+	unsigned long old_bytes = 0, new_bytes = 0;
 
-	// Read the object headers first so an oversized blob is never
-	// inflated into memory just to be diffed. Report it as binary,
-	// which suppresses inlining the same way max-blob-size does for
-	// the blob, plain and tree views.
+	// Read the object headers first so an oversized blob is never inflated
+	// into memory just to be diffed. Reporting it as binary suppresses
+	// inlining the same way max-blob-size does in the other views.
 	if (!is_null_oid(old_oid) &&
-	    odb_read_object_info(the_repository->objects, old_oid, &size1) < 0)
+	    odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0)
 		return 1;
 	if (!is_null_oid(new_oid) &&
-	    odb_read_object_info(the_repository->objects, new_oid, &size2) < 0)
+	    odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0)
 		return 1;
 
-	*old_size = size1;
-	*new_size = size2;
+	*old_size = old_bytes;
+	*new_size = new_bytes;
 
 	if (ctx.cfg.max_blob_size &&
-	    (size1 / 1024 > (unsigned long)ctx.cfg.max_blob_size ||
-	     size2 / 1024 > (unsigned long)ctx.cfg.max_blob_size)) {
+	    (old_bytes / 1024 > (unsigned long)ctx.cfg.max_blob_size ||
+	     new_bytes / 1024 > (unsigned long)ctx.cfg.max_blob_size)) {
 		*binary = 1;
 		return 0;
 	}
 
-	if (!load_mmfile(&file1, old_oid) || !load_mmfile(&file2, new_oid))
+	if (!load_mmfile(&old_file, old_oid))
+		return 1;
+	if (!load_mmfile(&new_file, new_oid)) {
+		release_mmfile(&old_file, old_oid);
 		return 1;
+	}
 
-	if ((file1.ptr && buffer_is_binary(file1.ptr, file1.size)) ||
-	    (file2.ptr && buffer_is_binary(file2.ptr, file2.size))) {
+	if (buffer_is_binary(old_file.ptr, old_file.size) ||
+	    buffer_is_binary(new_file.ptr, new_file.size)) {
 		*binary = 1;
-		if (file1.size)
-			free(file1.ptr);
-		if (file2.size)
-			free(file2.ptr);
+		release_mmfile(&old_file, old_oid);
+		release_mmfile(&new_file, new_oid);
 		return 0;
 	}
 
@@ -357,15 +413,13 @@ int cgit_diff_files(const struct object_id *old_oid,
 	diff_params.flags = XDF_NEED_MINIMAL;
 	if (ignorews)
 		diff_params.flags |= XDF_IGNORE_WHITESPACE;
-	emit_params.ctxlen = context > 0 ? context : 3;
+	emit_params.ctxlen = context > 0 ? context : DEFAULT_DIFF_CONTEXT;
 	emit_params.flags = XDL_EMIT_FUNCNAMES;
-	emit_cb.out_line = filediff_cb;
+	emit_cb.out_line = emit_line;
 	emit_cb.priv = fn;
-	xdl_diff(&file1, &file2, &diff_params, &emit_params, &emit_cb);
-	if (file1.size)
-		free(file1.ptr);
-	if (file2.size)
-		free(file2.ptr);
+	xdl_diff(&old_file, &new_file, &diff_params, &emit_params, &emit_cb);
+	release_mmfile(&old_file, old_oid);
+	release_mmfile(&new_file, new_oid);
 	return 0;
 }
 
@@ -412,41 +466,10 @@ void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char *prefix)
 		       ctx.qry.ignorews);
 }
 
-int cgit_parse_snapshots_mask(const char *str)
-{
-	struct string_list tokens = STRING_LIST_INIT_DUP;
-	struct string_list_item *item;
-	const struct cgit_snapshot_format *f;
-	int rv = 0;
-
-	/* favor legacy setting */
-	if (atoi(str))
-		return 1;
-
-	if (strcmp(str, "all") == 0)
-		return INT_MAX;
-
-	string_list_split(&tokens, str, " ", -1);
-	string_list_remove_empty_items(&tokens, 0);
-
-	for_each_string_list_item(item, &tokens) {
-		for (f = cgit_snapshot_formats; f->suffix; f++) {
-			if (!strcmp(item->string, f->suffix) ||
-			    !strcmp(item->string, f->suffix + 1)) {
-				rv |= cgit_snapshot_format_bit(f);
-				break;
-			}
-		}
-	}
-
-	string_list_clear(&tokens, 0);
-	return rv;
-}
-
 /*
- * Git's parse_date_format() dies on anything it does not recognize, which
- * would turn a typo in cgitrc into a failed request, so gate it behind the
- * set of formats cgit documents and leave the mode alone otherwise.
+ * 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.
  */
 void cgit_parse_date_format(const char *format, struct date_mode *mode)
 {
@@ -459,7 +482,7 @@ void cgit_parse_date_format(const char *format, struct date_mode *mode)
 	size_t i;
 
 	if (skip_prefix(format, "format:", &rest)) {
-		// an empty strftime format would render every date blank
+		// An empty strftime format would render every date blank.
 		if (*rest)
 			parse_date_format(format, mode);
 		return;
@@ -468,7 +491,7 @@ void cgit_parse_date_format(const char *format, struct date_mode *mode)
 	for (i = 0; i < ARRAY_SIZE(names); i++) {
 		if (!skip_prefix(format, names[i], &rest))
 			continue;
-		// "iso" also prefixes "iso8601", so keep looking on a partial hit
+		// "iso" also prefixes "iso8601", so a partial hit is skipped.
 		if (*rest && strcmp(rest, "-local"))
 			continue;
 		parse_date_format(format, mode);
@@ -476,14 +499,9 @@ void cgit_parse_date_format(const char *format, struct date_mode *mode)
 	}
 }
 
-typedef struct {
-	const char *name;
-	const char *value;
-} cgit_env_var;
-
-void cgit_prepare_repo_env(struct cgit_repo * repo)
+void cgit_prepare_repo_env(struct cgit_repo *repo)
 {
-	cgit_env_var env_vars[] = {
+	const env_var vars[] = {
 		{ .name = "CGIT_REPO_URL", .value = repo->url },
 		{ .name = "CGIT_REPO_NAME", .value = repo->name },
 		{ .name = "CGIT_REPO_PATH", .value = repo->path },
@@ -492,24 +510,18 @@ void cgit_prepare_repo_env(struct cgit_repo * repo)
 		{ .name = "CGIT_REPO_SECTION", .value = repo->section },
 		{ .name = "CGIT_REPO_CLONE_URL", .value = repo->clone_url }
 	};
-	int env_var_count = ARRAY_SIZE(env_vars);
-	cgit_env_var *p, *q;
-	static const char *warn = "cgit warning: failed to set env: %s=%s\n";
+	size_t i;
 
-	p = env_vars;
-	q = p + env_var_count;
-	for (; p < q; p++)
-		if (p->value && setenv(p->name, p->value, 1))
-			fprintf(stderr, warn, p->name, p->value);
+	for (i = 0; i < ARRAY_SIZE(vars); i++)
+		if (vars[i].value && setenv(vars[i].name, vars[i].value, 1))
+			fprintf(stderr,
+				"cgit warning: failed to set env: %s=%s\n",
+				vars[i].name, vars[i].value);
 }
 
-/* Read the content of the specified file into a newly allocated buffer,
- * zeroterminate the buffer, truncate at a new line, and return 0 on success,
- * errno otherwise.
- */
 int cgit_read_first_line(const char *path, char **buf, size_t *size)
 {
-	int fd, e;
+	int fd, err;
 	ssize_t got;
 	struct stat st;
 
@@ -517,9 +529,9 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 	if (fd == -1)
 		return errno;
 	if (fstat(fd, &st)) {
-		e = errno;
+		err = errno;
 		close(fd);
-		return e;
+		return err;
 	}
 	if (!S_ISREG(st.st_mode)) {
 		close(fd);
@@ -527,94 +539,66 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 	}
 	*buf = xmalloc(st.st_size + 1);
 	got = read_in_full(fd, *buf, st.st_size);
-	e = errno;
+	err = errno;
 	if (got < 0) {
 		free(*buf);
 		*buf = NULL;
 		*size = 0;
 		close(fd);
-		return e;
+		return err;
 	}
 	*size = got;
 	(*buf)[*size] = '\0';
 	*strchrnul(*buf, '\n') = '\0';
 	close(fd);
-	return (*size == (size_t)st.st_size ? 0 : e);
+	return (*size == (size_t)st.st_size ? 0 : err);
 }
 
-char *cgit_strdup_first_line(const char *txt)
+char *cgit_strdup_first_line(const char *text)
 {
-	char *t = xstrdup(txt);
-	*strchrnul(t, '\n') = '\0';
-	return t;
+	char *line = xstrdup(text);
+	*strchrnul(line, '\n') = '\0';
+	return line;
 }
 
-static int is_token_char(char c)
-{
-	return isalnum(c) || c == '_';
-}
-
-/* Replace name with getenv(name), return pointer to zero-terminating char
- */
-static char *expand_macro(char *name, size_t maxlength)
-{
-	char *value;
-	size_t len;
-
-	len = 0;
-	value = getenv(name);
-	if (value) {
-		len = strlen(value) + 1;
-		if (len > maxlength)
-			len = maxlength;
-		strlcpy(name, value, len);
-		--len;
-	}
-	return name + len;
-}
-
-/* Replace all tokens prefixed by '$' in the specified text with the
- * value of the named environment variable.
- * NB: the return value is a static buffer, i.e. it must be strdup'd
- * by the caller.
- */
-char *cgit_expand_macros(const char *txt)
+char *cgit_expand_macros(const char *text)
 {
 	static char result[MACRO_EXPANSION_BUFSIZE];
-	char *p, *start;
-	size_t len;
+	char *limit = result + MACRO_EXPANSION_BUFSIZE - 1;
+	char *out, *start;
 
-	p = result;
+	out = result;
 	start = NULL;
-	while (p < result + MACRO_EXPANSION_BUFSIZE - 1 && txt && *txt) {
-		*p = *txt;
+	while (out < limit && text && *text) {
+		*out = *text;
 		if (start) {
-			if (!is_token_char(*txt)) {
-				if (p - start > 0) {
-					*p = '\0';
-					len = result + MACRO_EXPANSION_BUFSIZE - start - 1;
-					p = expand_macro(start, len) - 1;
+			if (!is_token_char(*text)) {
+				if (out - start > 0) {
+					*out = '\0';
+					out = expand_macro(start, limit - start) - 1;
 				}
 				start = NULL;
-				txt--;
+				// Step back so the character that ended the
+				// token is written again past the expansion,
+				// where it may open a token of its own.
+				text--;
 			}
-			p++;
-			txt++;
+			out++;
+			text++;
 			continue;
 		}
-		if (*txt == '$') {
-			start = p;
-			txt++;
+		if (*text == '$') {
+			start = out;
+			text++;
 			continue;
 		}
-		p++;
-		txt++;
+		out++;
+		text++;
 	}
-	*p = '\0';
-	if (start && p - start > 0) {
-		len = result + MACRO_EXPANSION_BUFSIZE - start - 1;
-		p = expand_macro(start, len);
-		*p = '\0';
+	*out = '\0';
+	if (start && out - start > 0) {
+		out = expand_macro(start, limit - start);
+		*out = '\0';
 	}
 	return result;
 }
@@ -622,11 +606,11 @@ char *cgit_expand_macros(const char *txt)
 char *cgit_get_mimetype_for_filename(const char *filename)
 {
 	const char *ext;
-	char *mimetype, line[1024];
-	struct string_list list = STRING_LIST_INIT_NODUP;
+	char *type, line[1024];
+	struct string_list fields = STRING_LIST_INIT_NODUP;
 	size_t i;
 	FILE *file;
-	struct string_list_item *mime;
+	struct string_list_item *entry;
 
 	if (!filename)
 		return NULL;
@@ -637,9 +621,9 @@ char *cgit_get_mimetype_for_filename(const char *filename)
 	++ext;
 	if (!ext[0])
 		return NULL;
-	mime = string_list_lookup(&ctx.cfg.mimetypes, ext);
-	if (mime)
-		return xstrdup(mime->util);
+	entry = string_list_lookup(&ctx.cfg.mimetypes, ext);
+	if (entry)
+		return xstrdup(entry->util);
 
 	if (!ctx.cfg.mimetype_file)
 		return NULL;
@@ -649,16 +633,18 @@ char *cgit_get_mimetype_for_filename(const char *filename)
 	while (fgets(line, sizeof(line), file)) {
 		if (!line[0] || line[0] == '#')
 			continue;
-		string_list_split_in_place(&list, line, " \t\r\n", -1);
-		string_list_remove_empty_items(&list, 0);
-		mimetype = list.items[0].string;
-		for (i = 1; i < list.nr; i++) {
-			if (!strcasecmp(ext, list.items[i].string)) {
+		// A line of a mime.types file is one type followed by every
+		// extension that maps to it.
+		string_list_split_in_place(&fields, line, " \t\r\n", -1);
+		string_list_remove_empty_items(&fields, 0);
+		type = fields.items[0].string;
+		for (i = 1; i < fields.nr; i++) {
+			if (!strcasecmp(ext, fields.items[i].string)) {
 				fclose(file);
-				return xstrdup(mimetype);
+				return xstrdup(type);
 			}
 		}
-		string_list_clear(&list, 0);
+		string_list_clear(&fields, 0);
 	}
 	fclose(file);
 	return NULL;