[hackers] [quark] Reduce global state by localizing the server-struct || Laslo Hunhold

From: <git_AT_suckless.org>
Date: Mon, 17 Aug 2020 11:40:34 +0200 (CEST)

commit 65600ffe7a2868e95cf172550c85aa074e209e0d
Author: Laslo Hunhold <dev_AT_frign.de>
AuthorDate: Mon Aug 17 11:37:25 2020 +0200
Commit: Laslo Hunhold <dev_AT_frign.de>
CommitDate: Mon Aug 17 11:37:25 2020 +0200

    Reduce global state by localizing the server-struct
    
    The server-struct variable s was global, which made it readable and
    modifiable from any point in the code. Making it a local variable in
    main() instead and passing it as a pointer to constant memory to each
    function needing it makes much more sense and allows the compiler to
    warn us if we do try to modify it, which it wouldn't have before.
    
    Signed-off-by: Laslo Hunhold <dev_AT_frign.de>

diff --git a/http.c b/http.c
index 6bf4264..c9cf4dc 100644
--- a/http.c
+++ b/http.c
_AT_@ -528,7 +528,7 @@ parse_range(const char *str, size_t size, size_t *lower, size_t *upper)
 #define RELPATH(x) ((!*(x) || !strcmp(x, "/")) ? "." : ((x) + 1))
 
 enum status
-http_send_response(int fd, const struct request *req)
+http_send_response(int fd, const struct request *req, const struct server *s)
 {
         enum status returnstatus;
         struct in6_addr addr;
_AT_@ -547,27 +547,27 @@ http_send_response(int fd, const struct request *req)
 
         /* match vhost */
         vhostmatch = NULL;
- if (s.vhost) {
- for (i = 0; i < s.vhost_len; i++) {
+ if (s->vhost) {
+ for (i = 0; i < s->vhost_len; i++) {
                         /* switch to vhost directory if there is a match */
- if (!regexec(&s.vhost[i].re, req->field[REQ_HOST], 0,
+ if (!regexec(&(s->vhost[i].re), req->field[REQ_HOST], 0,
                                      NULL, 0)) {
- if (chdir(s.vhost[i].dir) < 0) {
+ if (chdir(s->vhost[i].dir) < 0) {
                                         return http_send_status(fd, (errno == EACCES) ?
                                                                 S_FORBIDDEN : S_NOT_FOUND);
                                 }
- vhostmatch = s.vhost[i].chost;
+ vhostmatch = s->vhost[i].chost;
                                 break;
                         }
                 }
- if (i == s.vhost_len) {
+ if (i == s->vhost_len) {
                         return http_send_status(fd, S_NOT_FOUND);
                 }
 
                 /* if we have a vhost prefix, prepend it to the target */
- if (s.vhost[i].prefix) {
+ if (s->vhost[i].prefix) {
                         if (esnprintf(tmptarget, sizeof(tmptarget), "%s%s",
- s.vhost[i].prefix, realtarget)) {
+ s->vhost[i].prefix, realtarget)) {
                                 return http_send_status(fd, S_REQUEST_TOO_LARGE);
                         }
                         memcpy(realtarget, tmptarget, sizeof(realtarget));
_AT_@ -575,19 +575,19 @@ http_send_response(int fd, const struct request *req)
         }
 
         /* apply target prefix mapping */
- for (i = 0; i < s.map_len; i++) {
- len = strlen(s.map[i].from);
- if (!strncmp(realtarget, s.map[i].from, len)) {
+ for (i = 0; i < s->map_len; i++) {
+ len = strlen(s->map[i].from);
+ if (!strncmp(realtarget, s->map[i].from, len)) {
                         /* match canonical host if vhosts are enabled and
                          * the mapping specifies a canonical host */
- if (s.vhost && s.map[i].chost &&
- strcmp(s.map[i].chost, vhostmatch)) {
+ if (s->vhost && s->map[i].chost &&
+ strcmp(s->map[i].chost, vhostmatch)) {
                                 continue;
                         }
 
                         /* swap out target prefix */
                         if (esnprintf(tmptarget, sizeof(tmptarget), "%s%s",
- s.map[i].to, realtarget + len)) {
+ s->map[i].to, realtarget + len)) {
                                 return http_send_status(fd, S_REQUEST_TOO_LARGE);
                         }
                         memcpy(realtarget, tmptarget, sizeof(realtarget));
_AT_@ -628,7 +628,7 @@ http_send_response(int fd, const struct request *req)
         }
 
         /* redirect if targets differ, host is non-canonical or we prefixed */
- if (strcmp(req->target, realtarget) || (s.vhost && vhostmatch &&
+ if (strcmp(req->target, realtarget) || (s->vhost && vhostmatch &&
             strcmp(req->field[REQ_HOST], vhostmatch))) {
                 res.status = S_MOVED_PERMANENTLY;
 
_AT_@ -636,14 +636,14 @@ http_send_response(int fd, const struct request *req)
                 encode(realtarget, tmptarget);
 
                 /* determine target location */
- if (s.vhost) {
+ if (s->vhost) {
                         /* absolute redirection URL */
                         targethost = req->field[REQ_HOST][0] ? vhostmatch ?
- vhostmatch : req->field[REQ_HOST] : s.host ?
- s.host : "localhost";
+ vhostmatch : req->field[REQ_HOST] : s->host ?
+ s->host : "localhost";
 
                         /* do we need to add a port to the Location? */
- hasport = s.port && strcmp(s.port, "80");
+ hasport = s->port && strcmp(s->port, "80");
 
                         /* RFC 2732 specifies to use brackets for IPv6-addresses
                          * in URLs, so we need to check if our host is one and
_AT_@ -661,7 +661,7 @@ http_send_response(int fd, const struct request *req)
                                       ipv6host ? "[" : "",
                                       targethost,
                                       ipv6host ? "]" : "", hasport ? ":" : "",
- hasport ? s.port : "", tmptarget)) {
+ hasport ? s->port : "", tmptarget)) {
                                 return http_send_status(fd, S_REQUEST_TOO_LARGE);
                         }
                 } else {
_AT_@ -679,16 +679,16 @@ http_send_response(int fd, const struct request *req)
         if (S_ISDIR(st.st_mode)) {
                 /* append docindex to target */
                 if (esnprintf(realtarget, sizeof(realtarget), "%s%s",
- req->target, s.docindex)) {
+ req->target, s->docindex)) {
                         return http_send_status(fd, S_REQUEST_TOO_LARGE);
                 }
 
                 /* stat the docindex, which must be a regular file */
                 if (stat(RELPATH(realtarget), &st) < 0 || !S_ISREG(st.st_mode)) {
- if (s.listdirs) {
+ if (s->listdirs) {
                                 /* remove index suffix and serve dir */
                                 realtarget[strlen(realtarget) -
- strlen(s.docindex)] = '\0';
+ strlen(s->docindex)] = '\0';
                                 return resp_dir(fd, RELPATH(realtarget), req);
                         } else {
                                 /* reject */
diff --git a/http.h b/http.h
index b6ba073..563d9fd 100644
--- a/http.h
+++ b/http.h
_AT_@ -4,6 +4,8 @@
 
 #include <limits.h>
 
+#include "util.h"
+
 #define HEADER_MAX 4096
 #define FIELD_MAX 200
 
_AT_@ -69,6 +71,7 @@ struct response {
 enum status http_send_header(int, const struct response *);
 enum status http_send_status(int, enum status);
 int http_get_request(int, struct request *);
-enum status http_send_response(int, const struct request *);
+enum status http_send_response(int, const struct request *,
+ const struct server *);
 
 #endif /* HTTP_H */
diff --git a/main.c b/main.c
index 0542fab..ddd8468 100644
--- a/main.c
+++ b/main.c
_AT_@ -23,7 +23,7 @@
 static char *udsname;
 
 static void
-serve(int infd, const struct sockaddr_storage *in_sa)
+serve(int infd, const struct sockaddr_storage *in_sa, const struct server *s)
 {
         struct request req;
         time_t t;
_AT_@ -38,7 +38,7 @@ serve(int infd, const struct sockaddr_storage *in_sa)
 
         /* handle request */
         if (!(status = http_get_request(infd, &req))) {
- status = http_send_response(infd, &req);
+ status = http_send_response(infd, &req, s);
         }
 
         /* write output to log */
_AT_@ -177,6 +177,9 @@ main(int argc, char *argv[])
         struct group *grp = NULL;
         struct passwd *pwd = NULL;
         struct rlimit rlim;
+ struct server s = {
+ .docindex = "index.html",
+ };
         struct sockaddr_storage in_sa;
         size_t i;
         socklen_t in_sa_len;
_AT_@ -190,13 +193,6 @@ main(int argc, char *argv[])
         char *user = "nobody";
         char *group = "nogroup";
 
- s.host = s.port = NULL;
- s.vhost = NULL;
- s.map = NULL;
- s.vhost_len = s.map_len = 0;
- s.docindex = "index.html";
- s.listdirs = 0;
-
         ARGBEGIN {
         case 'd':
                 servedir = EARGF(usage());
_AT_@ -372,7 +368,7 @@ main(int argc, char *argv[])
                         /* fork and handle */
                         switch (fork()) {
                         case 0:
- serve(infd, &in_sa);
+ serve(infd, &in_sa, &s);
                                 exit(0);
                                 break;
                         case -1:
diff --git a/util.c b/util.c
index 7ad512f..518a3eb 100644
--- a/util.c
+++ b/util.c
_AT_@ -16,7 +16,6 @@
 #include "util.h"
 
 char *argv0;
-struct server s;
 
 static void
 verr(const char *fmt, va_list ap)
diff --git a/util.h b/util.h
index b23a192..21c18f8 100644
--- a/util.h
+++ b/util.h
_AT_@ -23,7 +23,7 @@ struct map {
         char *to;
 };
 
-extern struct server {
+struct server {
         char *host;
         char *port;
         char *docindex;
_AT_@ -32,7 +32,7 @@ extern struct server {
         size_t vhost_len;
         struct map *map;
         size_t map_len;
-} s;
+};
 
 #undef MIN
 #define MIN(x,y) ((x) < (y) ? (x) : (y))
Received on Mon Aug 17 2020 - 11:40:34 CEST

This archive was generated by hypermail 2.3.0 : Mon Aug 17 2020 - 11:48:39 CEST