Restyle the sources and fix the audit's findings

1 file changed, 135 insertions, 140 deletions

diff --git a/source/html.c b/source/html.c
index 6e023d1..58ccf3b 100644
--- a/source/html.c
+++ b/source/html.c
@@ -1,20 +1,22 @@
-/* html.c: helper functions for html output
- *
- * Copyright (C) 2006-2014 cgit Development Team <cgit@lists.zx2c4.com>
- *
- * Licensed under GNU General Public License v2
- *   (see LICENSE.txt for full license text)
+/*
+ * The output layer every cgit page is built with, holding the escaping rules
+ * 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.
  */
 
 #include "cgit.h"
 #include "html.h"
-#include "url.h"
+
 #define HTML_WRITE_BUFSIZE (64 * 1024)
-static char html_buf[HTML_WRITE_BUFSIZE];
-static size_t html_buflen;
 
-/* Percent-encoding of each character, except: a-zA-Z0-9!$()*,./:;@- */
-static const char* url_escape_table[256] = {
+// The percent encoding for each byte, with NULL marking the bytes a URL may
+// carry as themselves. Those are the letters, the digits, and !$()*,-./:;@[]_~
+static const char *url_escape_table[256] = {
 	"%00", "%01", "%02", "%03", "%04", "%05", "%06", "%07",
 	"%08", "%09", "%0a", "%0b", "%0c", "%0d", "%0e", "%0f",
 	"%10", "%11", "%12", "%13", "%14", "%15", "%16", "%17",
@@ -49,24 +51,42 @@ static const char* url_escape_table[256] = {
 	"%f8", "%f9", "%fa", "%fb", "%fc", "%fd", "%fe", "%ff"
 };
 
+static char out_buf[HTML_WRITE_BUFSIZE];
+static size_t out_len;
+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.
+	if (write_in_full(STDOUT_FILENO, data, size) < 0)
+		die_errno("write error on html output");
+}
+
+/*
+ * Format into one of a rotating set of static buffers, so that a few results
+ * can be alive at once, for example as several arguments to one call. The slot
+ * count has to stay a power of two for the wrap below.
+ */
 char *cgit_fmt(const char *format, ...)
 {
 	static char buf[8][1024];
-	static int bufidx;
+	static int slot;
 	int len;
 	va_list args;
 
-	bufidx++;
-	bufidx &= 7;
+	slot++;
+	slot &= ARRAY_SIZE(buf) - 1;
 
 	va_start(args, format);
-	len = vsnprintf(buf[bufidx], sizeof(buf[bufidx]), format, args);
+	len = vsnprintf(buf[slot], sizeof(buf[slot]), format, args);
 	va_end(args);
-	if (len < 0 || (size_t)len >= sizeof(buf[bufidx])) {
+	if (len < 0 || (size_t)len >= sizeof(buf[slot])) {
 		fprintf(stderr, "[html.c] string truncated: %s\n", format);
 		exit(1);
 	}
-	return buf[bufidx];
+	return buf[slot];
 }
 
 char *cgit_fmtalloc(const char *format, ...)
@@ -81,77 +101,44 @@ char *cgit_fmtalloc(const char *format, ...)
 	return strbuf_detach(&sb, NULL);
 }
 
-/*
- * Page output is collected here and written out in whole buffers. A page is
- * built from a great many small fragments, and writing each one cost a syscall
- * apiece: a tree listing spent more time entering the kernel than generating
- * anything.
- *
- * cgit does not own stdout by itself, so the buffer has to be emptied before
- * anything else writes there. Those points are a filter taking over stdout,
- * the header block that precedes output produced by git itself, the cache
- * slot being measured, and process exit.
- */
-static void html_write(const char *data, size_t size)
-{
-	// A blob, snapshot or 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.
-	if (write_in_full(STDOUT_FILENO, data, size) < 0)
-		die_errno("write error on html output");
-}
-
 void html_flush(void)
 {
-	size_t len = html_buflen;
+	size_t len = out_len;
 
 	if (!len)
 		return;
-	// Clear the length first: html_write can die, and the error page it
-	// produces would otherwise try to flush the same bytes again.
-	html_buflen = 0;
-	html_write(html_buf, len);
+	// Clear the length first, because write_out can die and the error page
+	// it produces would otherwise try to flush the same bytes again.
+	out_len = 0;
+	write_out(out_buf, len);
 }
 
-/*
- * While a capture is in effect, page output is collected into the caller's
- * buffer instead of being written. The diff view uses this to render a file's
- * body at the point it already has the file open, rather than walking the whole
- * tree a second time to produce what the diffstat above it has to be printed
- * before.
- *
- * A capture must not span anything that writes to stdout by another route, a
- * filter in particular, since that output would escape the capture.
- */
-static struct strbuf *html_capture;
-
 void html_capture_begin(struct strbuf *sb)
 {
 	html_flush();
-	html_capture = sb;
+	capture = sb;
 }
 
 void html_capture_end(void)
 {
-	html_capture = NULL;
+	capture = NULL;
 }
 
 void html_raw(const char *data, size_t size)
 {
-	if (html_capture) {
-		strbuf_add(html_capture, data, size);
+	if (capture) {
+		strbuf_add(capture, data, size);
 		return;
 	}
 	if (size >= HTML_WRITE_BUFSIZE) {
-		// Nothing is gained by copying a blob through the buffer.
 		html_flush();
-		html_write(data, size);
+		write_out(data, size);
 		return;
 	}
-	if (html_buflen + size > HTML_WRITE_BUFSIZE)
+	if (out_len + size > HTML_WRITE_BUFSIZE)
 		html_flush();
-	memcpy(html_buf + html_buflen, data, size);
-	html_buflen += size;
+	memcpy(out_buf + out_len, data, size);
+	out_len += size;
 }
 
 void html(const char *txt)
@@ -162,13 +149,13 @@ void html(const char *txt)
 void htmlf(const char *format, ...)
 {
 	va_list args;
-	struct strbuf buf = STRBUF_INIT;
+	struct strbuf sb = STRBUF_INIT;
 
 	va_start(args, format);
-	strbuf_vaddf(&buf, format, args);
+	strbuf_vaddf(&sb, format, args);
 	va_end(args);
-	html(buf.buf);
-	strbuf_release(&buf);
+	html(sb.buf);
+	strbuf_release(&sb);
 }
 
 void html_txtf(const char *format, ...)
@@ -182,14 +169,14 @@ void html_txtf(const char *format, ...)
 
 void html_vtxtf(const char *format, va_list ap)
 {
-	va_list cp;
-	struct strbuf buf = STRBUF_INIT;
+	va_list copy;
+	struct strbuf sb = STRBUF_INIT;
 
-	va_copy(cp, ap);
-	strbuf_vaddf(&buf, format, cp);
-	va_end(cp);
-	html_txt(buf.buf);
-	strbuf_release(&buf);
+	va_copy(copy, ap);
+	strbuf_vaddf(&sb, format, copy);
+	va_end(copy);
+	html_txt(sb.buf);
+	strbuf_release(&sb);
 }
 
 void html_txt(const char *txt)
@@ -200,40 +187,40 @@ void html_txt(const char *txt)
 
 ssize_t html_ntxt(const char *txt, size_t len)
 {
-	const char *t = txt;
-	ssize_t slen;
+	const char *p = txt;
+	ssize_t left;
 
 	if (len > SSIZE_MAX)
 		return -1;
 
-	slen = (ssize_t) len;
-	while (t && *t && slen--) {
-		int c = *t;
+	left = (ssize_t) len;
+	while (p && *p && left--) {
+		int c = *p;
 		if (c == '<' || c == '>' || c == '&') {
-			html_raw(txt, t - txt);
+			html_raw(txt, p - txt);
 			if (c == '>')
 				html("&gt;");
 			else if (c == '<')
 				html("&lt;");
 			else if (c == '&')
 				html("&amp;");
-			txt = t + 1;
+			txt = p + 1;
 		}
-		t++;
+		p++;
 	}
-	if (t != txt)
-		html_raw(txt, t - txt);
-	return slen;
+	if (p != txt)
+		html_raw(txt, p - txt);
+	return left;
 }
 
-void html_attrf(const char *fmt, ...)
+void html_attrf(const char *format, ...)
 {
-	va_list ap;
+	va_list args;
 	struct strbuf sb = STRBUF_INIT;
 
-	va_start(ap, fmt);
-	strbuf_vaddf(&sb, fmt, ap);
-	va_end(ap);
+	va_start(args, format);
+	strbuf_vaddf(&sb, format, args);
+	va_end(args);
 
 	html_attr(sb.buf);
 	strbuf_release(&sb);
@@ -241,11 +228,11 @@ void html_attrf(const char *fmt, ...)
 
 void html_attr(const char *txt)
 {
-	const char *t = txt;
-	while (t && *t) {
-		int c = *t;
+	const char *p = txt;
+	while (p && *p) {
+		int c = *p;
 		if (c == '<' || c == '>' || c == '\'' || c == '\"' || c == '&') {
-			html_raw(txt, t - txt);
+			html_raw(txt, p - txt);
 			if (c == '>')
 				html("&gt;");
 			else if (c == '<')
@@ -256,74 +243,76 @@ void html_attr(const char *txt)
 				html("&quot;");
 			else if (c == '&')
 				html("&amp;");
-			txt = t + 1;
+			txt = p + 1;
 		}
-		t++;
+		p++;
 	}
-	if (t != txt)
+	if (p != txt)
 		html(txt);
 }
 
 void html_url_path(const char *txt)
 {
-	const char *t = txt;
-	while (t && *t) {
-		unsigned char c = *t;
-		const char *e = url_escape_table[c];
-		if (e && c != '+' && c != '&') {
-			html_raw(txt, t - txt);
-			html(e);
-			txt = t + 1;
+	const char *p = txt;
+	while (p && *p) {
+		unsigned char c = *p;
+		const char *esc = url_escape_table[c];
+		// The table is shared with the query string case, where a plus
+		// means a space and an ampersand separates parameters. Neither
+		// carries that meaning in a path.
+		if (esc && c != '+' && c != '&') {
+			html_raw(txt, p - txt);
+			html(esc);
+			txt = p + 1;
 		}
-		t++;
+		p++;
 	}
-	if (t != txt)
+	if (p != txt)
 		html(txt);
 }
 
 void html_url_arg(const char *txt)
 {
-	const char *t = txt;
-	while (t && *t) {
-		unsigned char c = *t;
-		const char *e = url_escape_table[c];
+	const char *p = txt;
+	while (p && *p) {
+		unsigned char c = *p;
+		const char *esc = url_escape_table[c];
 		if (c == ' ')
-			e = "+";
-		if (e) {
-			html_raw(txt, t - txt);
-			html(e);
-			txt = t + 1;
+			esc = "+";
+		if (esc) {
+			html_raw(txt, p - txt);
+			html(esc);
+			txt = p + 1;
 		}
-		t++;
+		p++;
 	}
-	if (t != txt)
+	if (p != txt)
 		html(txt);
 }
 
 void html_header_arg_in_quotes(const char *txt)
 {
-	const char *t = txt;
-	while (t && *t) {
-		unsigned char c = *t;
-		const char *e = NULL;
+	const char *p = txt;
+	while (p && *p) {
+		unsigned char c = *p;
+		const char *esc = NULL;
 		if (c == '\\')
-			e = "\\\\";
+			esc = "\\\\";
 		else if (c == '\r')
-			e = "\\r";
+			esc = "\\r";
 		else if (c == '\n')
-			e = "\\n";
+			esc = "\\n";
 		else if (c == '"')
-			e = "\\\"";
-		if (e) {
-			html_raw(txt, t - txt);
-			html(e);
-			txt = t + 1;
+			esc = "\\\"";
+		if (esc) {
+			html_raw(txt, p - txt);
+			html(esc);
+			txt = p + 1;
 		}
-		t++;
+		p++;
 	}
-	if (t != txt)
+	if (p != txt)
 		html(txt);
-
 }
 
 void html_hidden(const char *name, const char *value)
@@ -335,7 +324,8 @@ void html_hidden(const char *name, const char *value)
 	html("'/>");
 }
 
-void html_option(const char *value, const char *text, const char *selected_value)
+void html_option(const char *value, const char *text,
+		 const char *selected_value)
 {
 	html("<option value='");
 	html_attr(value);
@@ -375,6 +365,10 @@ void html_link_close(void)
 	html("</a>");
 }
 
+/*
+ * Render one permission triplet, so a caller prints a whole mode by passing it
+ * shifted right by six, then by three, then unshifted.
+ */
 void html_fileperm(unsigned short mode)
 {
 	htmlf("%c%c%c", (mode & 4 ? 'r' : '-'),
@@ -392,20 +386,21 @@ int html_include(const char *filename)
 			filename, strerror(errno), errno);
 		return -1;
 	}
-	while ((len = fread(buf, 1, 4096, f)) > 0)
+	while ((len = fread(buf, 1, sizeof(buf), f)) > 0)
 		html_raw(buf, len);
 	fclose(f);
 	return 0;
 }
 
-void http_parse_querystring(const char *txt, void (*fn)(const char *name, const char *value))
+void http_parse_querystring(const char *txt,
+			    void (*fn)(const char *name, const char *value))
 {
-	const char *t = txt;
+	const char *p = txt;
 
-	while (t && *t) {
-		char *name = url_decode_parameter_name(&t);
+	while (p && *p) {
+		char *name = url_decode_parameter_name(&p);
 		if (*name) {
-			char *value = url_decode_parameter_value(&t);
+			char *value = url_decode_parameter_value(&p);
 			fn(name, value);
 			free(value);
 		}