Hello fellow hackers,

thanks to Anselm I was given the trust of maintainership for the
quark webserver.
In the future I hope that there will be more active development on this
fine piece of software.

A few smaller commits already came in, but, as suggested by sin, I think
it's a good idea opening commits for discussion when they are slightly
heavier.

This commit rids quark of duplicate code and centralizes response
entries. Read more in the commit message.

Let me know what you think. In case it's widely accepted, I'll apply it
asap.

Cheers

FRIGN

-- 
FRIGN <d...@frign.de>
>From 0c6e10549ea354e5cb267d20b60a727bb06101c7 Mon Sep 17 00:00:00 2001
From: FRIGN <d...@frign.de>
Date: Wed, 6 Aug 2014 17:24:05 +0200
Subject: [PATCH] Refactor response-constructors

Instead of providing a function for each entry-type, use a small
static lookup-table and one function to rule them all.
In the future, in case the list grows, we might think about
implementing it with a small hash-lookup, but currently,
it's easy enough synchronizing the enum and the array.

While at it, improve the logic in the code itself
by using logical OR's instead of AND's.
---
 quark.c | 124 ++++++++++++++++++++--------------------------------------------
 1 file changed, 38 insertions(+), 86 deletions(-)

diff --git a/quark.c b/quark.c
index 917b3d4..f189993 100644
--- a/quark.c
+++ b/quark.c
@@ -46,6 +46,21 @@ static const char HttpUnauthorized[] = "401 Unauthorized";
 static const char HttpNotFound[]     = "404 Not Found";
 static const char texthtml[]         = "text/html";
 
+enum {
+	HEADER,
+	CONTENTLEN,
+	LOCATION,
+	CONTENTTYPE,
+	MODIFIED
+};
+
+static const char *resentry[] = {
+	"HTTP/1.1 %s\r\nConnection: close\r\nDate: %s\r\nServer: quark-"VERSION"\r\n",
+	"Content-Length: %lu\r\n",
+	"Location: %s%s\r\n",
+	"Content-Type: %s\r\n",
+	"Last-Modified: %s\r\n" };
+
 static ssize_t writetext(const char *buf);
 static ssize_t writedata(const char *buf, size_t buflen);
 static void atomiclog(int fd, const char *errstr, va_list ap);
@@ -140,68 +155,17 @@ die(const char *errstr, ...) {
 }
 
 int
-responsehdr(const char *status) {
-	if(snprintf(resbuf, MAXBUFLEN,
-		"HTTP/1.1 %s\r\n"
-		"Connection: close\r\n"
-		"Date: %s\r\n"
-		"Server: quark-"VERSION"\r\n",
-		status, tstamp()) >= MAXBUFLEN)
-	{
-		logerrmsg("snprintf failed, buffer size exceeded");
-		return -1;
-	}
-	return writetext(resbuf);
-}
-
-int
-responsecontentlen(off_t size) {
-	if(snprintf(resbuf, MAXBUFLEN,
-		"Content-Length: %lu\r\n",
-		size) >= MAXBUFLEN)
-	{
-		logerrmsg("snprintf failed, buffer sizeof exceeded");
-		return -1;
-	}
-	return writetext(resbuf);
-}
-
-int
-responselocation(const char *location, const char *pathinfo) {
-	if(snprintf(resbuf, MAXBUFLEN,
-		"Location: %s%s\r\n",
-		location, pathinfo) >= MAXBUFLEN)
-	{
-		logerrmsg("snprintf failed, buffer sizeof exceeded");
-		return -1;
-	}
-	return writetext(resbuf);
-}
+putresentry(int type, ...) {
+	va_list ap;
 
-int
-responsecontenttype(const char *mimetype) {
-	if(snprintf(resbuf, MAXBUFLEN,
-		"Content-Type: %s\r\n",
-		mimetype) >= MAXBUFLEN)
-	{
-		logerrmsg("snprintf failed, buffer sizeof exceeded");
-		return -1;
+	va_start(ap, type);
+	if(vsnprintf(resbuf, MAXBUFLEN, resentry[type], ap) >= MAXBUFLEN) {
+		logerrmsg("vsnprintf failed, buffer size exceeded");
 	}
+	va_end(ap);
 	return writetext(resbuf);
 }
 
-int
-responsemodified(char *mod) {
-    if(snprintf(resbuf, MAXBUFLEN,
-        "Last-Modified: %s\r\n",
-        mod) >= MAXBUFLEN)
-    {
-        logerrmsg("snprintf failed, buffer sizeof exceeded");
-        return -1;
-    }
-    return writetext(resbuf);
-}
-
 void
 responsefiledata(int fd, off_t size) {
 	char buf[BUFSIZ];
@@ -226,10 +190,8 @@ responsefile(void) {
 	r = stat(reqbuf, &st);
 	if(r == -1 || (ffd = open(reqbuf, O_RDONLY)) == -1) {
 		logerrmsg("%s requests unknown path %s\n", host, reqbuf);
-		if(responsehdr(HttpNotFound) != -1
-		&& responsecontenttype(texthtml) != -1)
-			;
-		else
+		if(putresentry(HEADER, HttpNotFound, tstamp()) == -1
+		|| putresentry(CONTENTTYPE, texthtml) == -1)
 			return;
 		if(req.type == GET)
 			writetext("\r\n<html><body>404 Not Found</body></html>\r\n");
@@ -239,9 +201,7 @@ responsefile(void) {
 		memcpy(mod, asctime(gmtime(&t)), 24);
 		mod[24] = 0;
 		if(!strcmp(reqmod, mod)) {
-			if(responsehdr(HttpNotModified) != -1)
-				;
-			else
+			if(putresentry(HEADER, HttpNotModified, tstamp()) == -1)
 				return;
 		} else {
 			if((p = strrchr(reqbuf, '.'))) {
@@ -252,12 +212,10 @@ responsefile(void) {
 						break;
 					}
 			}
-			if(responsehdr(HttpOk) != -1
-			&& responsemodified(mod) != -1
-			&& responsecontentlen(st.st_size) != -1
-			&& responsecontenttype(mimetype) != -1)
-				;
-			else
+			if(putresentry(HEADER, HttpOk, tstamp()) == -1
+			|| putresentry(MODIFIED, mod) == -1
+			|| putresentry(CONTENTLEN, st.st_size) == -1
+			|| putresentry(CONTENTTYPE, mimetype) == -1)
 				return;
 			if(req.type == GET && writetext("\r\n") != -1)
 				responsefiledata(ffd, st.st_size);
@@ -270,10 +228,8 @@ void
 responsedirdata(DIR *d) {
 	struct dirent *e;
 
-	if(responsehdr(HttpOk) != -1
-	&& responsecontenttype(texthtml) != -1)
-		;
-	else
+	if(putresentry(HEADER, HttpOk, tstamp()) == -1
+	|| putresentry(CONTENTTYPE, texthtml) == -1)
 		return;
 	if(req.type == GET) {
 		if(writetext("\r\n<html><body><a href='..'>..</a><br>\r\n") == -1)
@@ -304,11 +260,9 @@ responsedir(void) {
 		reqbuf[len++] = '/';
 		reqbuf[len] = 0;
 		logmsg("redirecting %s to %s%s\n", host, location, reqbuf);
-		if(responsehdr(HttpMoved) != -1
-		&& responselocation(location, reqbuf) != -1
-		&& responsecontenttype(texthtml) != -1)
-			;
-		else
+		if(putresentry(HEADER, HttpMoved, tstamp()) == -1
+		|| putresentry(LOCATION, location, reqbuf) == -1
+		|| putresentry(CONTENTTYPE, texthtml) == -1)
 			return;
 		if(req.type == GET)
 			writetext("\r\n<html><body>301 Moved Permanently</a></body></html>\r\n");
@@ -348,7 +302,7 @@ responsecgi(void) {
 	if(chdir(cgi_dir) == -1)
 		logerrmsg("chdir to cgi directory %s failed: %s\n", cgi_dir, strerror(errno));
 	if((cgi = popen(cgi_script, "r"))) {
-		if(responsehdr(HttpOk) == -1)
+		if(putresentry(HEADER, HttpOk, tstamp()) == -1)
 			return;
 		while((r = fread(resbuf, 1, MAXBUFLEN, cgi)) > 0) {
 			if(writedata(resbuf, r) == -1) {
@@ -360,10 +314,8 @@ responsecgi(void) {
 	}
 	else {
 		logerrmsg("%s requests %s, but cannot run cgi script %s\n", host, cgi_script, reqbuf);
-		if(responsehdr(HttpNotFound) != -1
-		&& responsecontenttype(texthtml) != -1)
-			;
-		else
+		if(putresentry(HEADER, HttpNotFound, tstamp()) == -1
+		|| putresentry(CONTENTTYPE, texthtml) == -1)
 			return;
 		if(req.type == GET)
 			writetext("\r\n<html><body>404 Not Found</body></html>\r\n");
@@ -378,8 +330,8 @@ response(void) {
 	for(p = reqbuf; *p; p++)
 		if(*p == '\\' || (*p == '/' && *(p + 1) == '.')) { /* don't serve bogus or hidden files */
 			logerrmsg("%s requests bogus or hidden file %s\n", host, reqbuf);
-			if(responsehdr(HttpUnauthorized) != -1
-			&& responsecontenttype(texthtml) != -1)
+			if(putresentry(HEADER, HttpUnauthorized, tstamp()) == -1
+			|| putresentry(CONTENTTYPE, texthtml) == -1)
 				;
 			else
 				return;
-- 
1.8.5.5

Reply via email to