Gather page output into one buffer

6 files changed, 67 insertions, 2 deletions

diff --git a/source/cache.c b/source/cache.cindex 105ecfe..e916728 100644--- a/source/cache.c+++ b/source/cache.c@@ -242,6 +242,7 @@ static int fill_slot(struct cache_slot *slot) 	slot->fn();  	/* Make sure any buffered data is flushed to the file */+	html_flush(); 	if (fflush(stdout)) 		return errno; 
diff --git a/source/cache.c b/source/cache.c
index 105ecfe..e916728 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -242,6 +242,7 @@ static int fill_slot(struct cache_slot *slot)
 	slot->fn();
 
 	/* Make sure any buffered data is flushed to the file */
+	html_flush();
 	if (fflush(stdout))
 		return errno;
 
diff --git a/source/cgit.c b/source/cgit.cindex 5cb52fd..2ecfb4f 100644--- a/source/cgit.c+++ b/source/cgit.c@@ -1087,6 +1087,9 @@ int cmd_main(int argc, const char **argv) 	isolate_git_environment(); 	cgit_init_filters(); 	atexit(cgit_cleanup_filters);+	// Registered second so it runs first, since exit is reached from error+	// paths and from the HEAD shortcut with a page still buffered.+	atexit(html_flush); 	set_die_routine(cgit_die_routine);  	prepare_context();
diff --git a/source/cgit.c b/source/cgit.c
index 5cb52fd..2ecfb4f 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -1087,6 +1087,9 @@ int cmd_main(int argc, const char **argv)
 	isolate_git_environment();
 	cgit_init_filters();
 	atexit(cgit_cleanup_filters);
+	// Registered second so it runs first, since exit is reached from error
+	// paths and from the HEAD shortcut with a page still buffered.
+	atexit(html_flush);
 	set_die_routine(cgit_die_routine);
 
 	prepare_context();
diff --git a/source/filter.c b/source/filter.cindex e15a5e9..f195534 100644--- a/source/filter.c+++ b/source/filter.c@@ -216,6 +216,10 @@ static inline int hook_lua_filter(lua_State *lua_state, void (*fn)(const char *t 	save_filter = current_write_filter; 	unhook_write(); 	fn(str);+	// fn buffers, so empty it while the hook is still off. Re-hooking first+	// would send the page's own bytes back into the filter that asked for+	// them to be written.+	html_flush(); 	hook_write(save_filter, save_filter_write);  	return 0;@@ -368,6 +372,9 @@ int cgit_open_filter(struct cgit_filter *filter, ...) 	va_list ap; 	if (!filter) 		return 0;+	// Whatever is still buffered belongs to the page, not to the filter+	// that is about to take over stdout.+	html_flush(); 	va_start(ap, filter); 	result = filter->open(filter, ap); 	va_end(ap);@@ -378,6 +385,9 @@ int cgit_close_filter(struct cgit_filter *filter) { 	if (!filter) 		return 0;+	// Anything written while the filter was open is meant for it, so this+	// has to go out before the filter hands stdout back.+	html_flush(); 	return filter->close(filter); } 
diff --git a/source/filter.c b/source/filter.c
index e15a5e9..f195534 100644
--- a/source/filter.c
+++ b/source/filter.c
@@ -216,6 +216,10 @@ static inline int hook_lua_filter(lua_State *lua_state, void (*fn)(const char *t
 	save_filter = current_write_filter;
 	unhook_write();
 	fn(str);
+	// fn buffers, so empty it while the hook is still off. Re-hooking first
+	// would send the page's own bytes back into the filter that asked for
+	// them to be written.
+	html_flush();
 	hook_write(save_filter, save_filter_write);
 
 	return 0;
@@ -368,6 +372,9 @@ int cgit_open_filter(struct cgit_filter *filter, ...)
 	va_list ap;
 	if (!filter)
 		return 0;
+	// Whatever is still buffered belongs to the page, not to the filter
+	// that is about to take over stdout.
+	html_flush();
 	va_start(ap, filter);
 	result = filter->open(filter, ap);
 	va_end(ap);
@@ -378,6 +385,9 @@ int cgit_close_filter(struct cgit_filter *filter)
 {
 	if (!filter)
 		return 0;
+	// Anything written while the filter was open is meant for it, so this
+	// has to go out before the filter hands stdout back.
+	html_flush();
 	return filter->close(filter);
 }
 
diff --git a/source/html.c b/source/html.cindex c3ab696..dbb0a57 100644--- a/source/html.c+++ b/source/html.c@@ -9,6 +9,9 @@ #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] = {@@ -78,12 +81,52 @@ char *cgit_fmtalloc(const char *format, ...) 	return strbuf_detach(&sb, NULL); } -void html_raw(const char *data, size_t size)+/*+ * 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) {-	if (write(STDOUT_FILENO, data, size) != 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;++	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);+}++void html_raw(const char *data, size_t size)+{+	if (size >= HTML_WRITE_BUFSIZE) {+		// Nothing is gained by copying a blob through the buffer.+		html_flush();+		html_write(data, size);+		return;+	}+	if (html_buflen + size > HTML_WRITE_BUFSIZE)+		html_flush();+	memcpy(html_buf + html_buflen, data, size);+	html_buflen += size;+}+ void html(const char *txt) { 	html_raw(txt, strlen(txt));
diff --git a/source/html.c b/source/html.c
index c3ab696..dbb0a57 100644
--- a/source/html.c
+++ b/source/html.c
@@ -9,6 +9,9 @@
 #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] = {
@@ -78,12 +81,52 @@ char *cgit_fmtalloc(const char *format, ...)
 	return strbuf_detach(&sb, NULL);
 }
 
-void html_raw(const char *data, size_t size)
+/*
+ * 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)
 {
-	if (write(STDOUT_FILENO, data, size) != 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;
+
+	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);
+}
+
+void html_raw(const char *data, size_t size)
+{
+	if (size >= HTML_WRITE_BUFSIZE) {
+		// Nothing is gained by copying a blob through the buffer.
+		html_flush();
+		html_write(data, size);
+		return;
+	}
+	if (html_buflen + size > HTML_WRITE_BUFSIZE)
+		html_flush();
+	memcpy(html_buf + html_buflen, data, size);
+	html_buflen += size;
+}
+
 void html(const char *txt)
 {
 	html_raw(txt, strlen(txt));
diff --git a/source/html.h b/source/html.hindex 7572623..353254e 100644--- a/source/html.h+++ b/source/html.h@@ -13,6 +13,10 @@ extern void html(const char *txt);  * blob limits do not bound. */ #define HTML_BATCH (64 * 1024) +/* Write out whatever page output is still buffered. Call before anything+ * other than html_raw writes to stdout. */+extern void html_flush(void);+ __attribute__((format (printf,1,2))) extern void htmlf(const char *format,...); 
diff --git a/source/html.h b/source/html.h
index 7572623..353254e 100644
--- a/source/html.h
+++ b/source/html.h
@@ -13,6 +13,10 @@ extern void html(const char *txt);
  * blob limits do not bound. */
 #define HTML_BATCH (64 * 1024)
 
+/* Write out whatever page output is still buffered. Call before anything
+ * other than html_raw writes to stdout. */
+extern void html_flush(void);
+
 __attribute__((format (printf,1,2)))
 extern void htmlf(const char *format,...);
 
diff --git a/source/ui-shared.c b/source/ui-shared.cindex 125a235..23da286 100644--- a/source/ui-shared.c+++ b/source/ui-shared.c@@ -808,6 +808,10 @@ void cgit_print_http_headers(void) 	if (ctx.page.etag) 		htmlf("ETag: \"%s\"\n", ctx.page.etag); 	html("\n");+	// Several pages follow the header block with output produced by git+	// rather than by html_raw, the patch, raw diff and snapshot views among+	// them. Emptying the buffer here keeps the headers ahead of it.+	html_flush(); 	if (ctx.env.request_method && !strcmp(ctx.env.request_method, "HEAD")) 		exit(0); }
diff --git a/source/ui-shared.c b/source/ui-shared.c
index 125a235..23da286 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -808,6 +808,10 @@ void cgit_print_http_headers(void)
 	if (ctx.page.etag)
 		htmlf("ETag: \"%s\"\n", ctx.page.etag);
 	html("\n");
+	// Several pages follow the header block with output produced by git
+	// rather than by html_raw, the patch, raw diff and snapshot views among
+	// them. Emptying the buffer here keeps the headers ahead of it.
+	html_flush();
 	if (ctx.env.request_method && !strcmp(ctx.env.request_method, "HEAD"))
 		exit(0);
 }