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