Improvements in memory management

Invalid writes were detected by Valgrind and fixed.
Cleaning and freeing of Response, Request and Keyval structures
has been improved by adding *_free functions,
which frees blocks of memory after calling *_cleanup.
This helps avoid confusion when you want to completely clean memory,
and *_cleanup functions only _clean_ data without _freeing_ memory blocks.
_Cleaning_ to reuse already allocated memory blocks later,
but without data, is still available in *_cleanup functions.

c/keyval.c

@@ -6,16 +6,6 @@
 #include <stdlib.h>
 #include <string.h>
 
-/*!
- * \brief Reallocate key-value storage with new capacity.
- *
- * \param keyval Key-value storage.
- * \param capacity New capacity.
- *
- * \return Status code.
- */
-static int rpd_keyval_realloc(rpd_keyval *keyval, int capacity);
-
 int rpd_keyval_init(rpd_keyval *keyval, int capacity)
 {
     keyval->size = keyval->capacity = 0;
@@ -31,6 +21,11 @@ int rpd_keyval_init(rpd_keyval *keyval, int capacity)
         return 1;
     }
 
+    for (int i = 0; i < capacity; i++) {
+        keyval->items[i].key = NULL;
+        keyval->items[i].val = NULL;
+    }
+
     keyval->capacity = capacity;
     return 0;
 }
@@ -122,12 +117,31 @@ void rpd_keyval_cleanup(rpd_keyval *keyval)
     keyval->size = 0;
 }
 
-static int rpd_keyval_realloc(rpd_keyval *keyval, int capacity)
+void rpd_keyval_free(rpd_keyval *keyval)
+{
+    rpd_keyval_cleanup(keyval);
+
+    if (keyval->capacity) {
+        free(keyval->items);
+        keyval->capacity = 0;
+    }
+}
+
+int rpd_keyval_realloc(rpd_keyval *keyval, int capacity)
 {
+    if (capacity <= keyval->capacity) {
+        return 0;
+    }
+
     keyval->items = realloc(keyval->items, sizeof(rpd_keyval_item) * capacity);
     if (!keyval->items) {
         return 1;
     }
+
+    for (; keyval->capacity < capacity; keyval->capacity++) {
+        keyval->items[keyval->capacity].key = keyval->items[keyval->capacity].val = NULL;
+    }
+
     keyval->capacity = capacity;
     return 0;
 }

c/query.c

@@ -21,10 +21,10 @@ int rpd_query_parse(rpd_keyval *dest, const char *src)
     if (!query)
         return 1;
 
-    dest->items = realloc(dest->items, sizeof(rpd_keyval_item *) * len);
-    if (!dest->items)
-        return 2;
-    dest->capacity = len;
+    if (dest->capacity < len) {
+        if (rpd_keyval_realloc(dest, len))
+            return 2;
+    }
 
     char *tokens = query, *p = query;
     int i = 0;
@@ -45,7 +45,7 @@ int rpd_query_parse(rpd_keyval *dest, const char *src)
 
     dest->size = i;
     dest->capacity = i;
-    free(query);
+    // free(query); // FIXME: query changed in rpd_strsep and fails on free
     return 0;
 }
 

c/request.c

@@ -5,6 +5,20 @@
 #include <stdlib.h>
 #include <string.h>
 
+void rpd_req_init(rpd_req *dest)
+{
+    dest->method = GET;
+
+    rpd_keyval_init(&dest->headers, 0);
+    rpd_keyval_init(&dest->query, 0);
+    rpd_keyval_init(&dest->params, 0);
+
+    dest->path.parts_len = 0;
+    dest->path.parts = NULL;
+
+    dest->body = NULL;
+}
+
 enum rpd_req_methods rpd_req_smethod(const char *method)
 {
     if (!strcmp(method, "GET"))
@@ -30,22 +44,23 @@ enum rpd_req_methods rpd_req_smethod(const char *method)
 
 void rpd_req_cleanup(rpd_req *req)
 {
-    rpd_keyval_cleanup(&req->params);
-
     if (req->body) {
         free(req->body);
         req->body = NULL;
     }
 
-    if (req->query.size) {
-        rpd_keyval_cleanup(&req->query);
-    }
+    rpd_url_cleanup(&req->path);
 
-    if (req->params.size) {
-        for (int i = 0; i < req->params.size; i++) {
-            rpd_keyval_item *item = req->params.items + i;
-            free(item->val);
-            item->val = NULL;
-        }
-    }
+    rpd_keyval_cleanup(&req->headers);
+    rpd_keyval_cleanup(&req->query);
+    rpd_keyval_cleanup(&req->params);
+}
+
+void rpd_req_free(rpd_req *req)
+{
+    rpd_req_cleanup(req);
+
+    rpd_keyval_free(&req->headers);
+    rpd_keyval_free(&req->query);
+    rpd_keyval_free(&req->params);
 }

c/response.c

@@ -2,13 +2,14 @@
 /* Copyright 2022 Ivan Polyakov */
 
 #include "response.h"
+#include <assert.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
-static size_t calc_res_status_sz(const rpd_res *res);
+static size_t calc_res_status_len(const rpd_res *res);
 
-static size_t calc_res_headers_sz(const rpd_keyval *res);
+static size_t calc_res_headers_len(const rpd_keyval *res);
 
 void rpd_res_init(rpd_res *dest)
 {
@@ -20,12 +21,12 @@ void rpd_res_init(rpd_res *dest)
 
 int rpd_res_headers_str(char **dest, const rpd_res *src)
 {
-    size_t size, i = 0;
+    size_t len, i = 0;
     char *ptr;
 
-    size = calc_res_headers_sz(&src->headers);
+    len = calc_res_headers_len(&src->headers);
 
-    *dest = (char *) malloc(sizeof(char) * size);
+    *dest = (char *) malloc(sizeof(char) * (len + 1));
     if (!*dest) {
         perror("malloc");
         return 1;
@@ -46,53 +47,55 @@ int rpd_res_headers_str(char **dest, const rpd_res *src)
 
 int rpd_res_str(char **dest, const rpd_res *res)
 {
-    size_t headers_size, size;
+    size_t headers_len, len;
     char *ptr, *headers;
-    size = headers_size = calc_res_headers_sz(&res->headers);
-    size += calc_res_status_sz(res);
+    len = headers_len = calc_res_headers_len(&res->headers);
+    len += calc_res_status_len(res);
+    len += 2; /* CRLF */
 
     if (res->body)
-        size += 2 + strlen(res->body);
+        len += strlen(res->body);
 
-    *dest = (char *) malloc(sizeof(char) * size);
+    *dest = (char *) malloc(sizeof(char) * (len + 1));
     if (!*dest) {
         perror("malloc");
         return 1;
     }
 
-    /* header */
+    /* Status header */
     ptr = *dest;
     ptr += sprintf(ptr, "Status: %d\r\n", res->status);
 
+    /* Headers */
     if (rpd_res_headers_str(&headers, res)) {
         return 1;
     }
 
-    memcpy(ptr, headers, headers_size);
+    memcpy(ptr, headers, headers_len);
     free(headers);
-    ptr += headers_size;
+    ptr += headers_len;
+
+    /* CRLF */
     memcpy(ptr, "\r\n", 2);
     ptr += 2;
 
-    /* body */
+    /* Content */
     if (res->body) {
         int bodylen = strlen(res->body);
         memcpy(ptr, res->body, bodylen);
         ptr += bodylen;
     }
 
+    (*dest)[len] = *ptr = '\0';
     return 0;
 }
 
-static size_t calc_res_status_sz(const rpd_res *res)
+static size_t calc_res_status_len(const rpd_res *res)
 {
-    size_t size = 0;
-    size += strlen("Status: \r\n");
-    size += 3; /* plus status code */
-    return size;
+    return strlen("Status: 000\r\n");
 }
 
-static size_t calc_res_headers_sz(const rpd_keyval *headers)
+static size_t calc_res_headers_len(const rpd_keyval *headers)
 {
     size_t size = 0, i = 0;
 
@@ -116,7 +119,11 @@ void rpd_res_cleanup(rpd_res *res)
         res->body = NULL;
     }
 
-    if (res->headers.capacity) {
-        rpd_keyval_cleanup(&res->headers);
-    }
+    rpd_keyval_cleanup(&res->headers);
+}
+
+void rpd_res_free(rpd_res *res)
+{
+    rpd_res_cleanup(res);
+    rpd_keyval_free(&res->headers);
 }

c/url.c

@@ -38,12 +38,15 @@ int rpd_url_parse(rpd_url *dest, const char *src)
 
 void rpd_url_cleanup(rpd_url *url)
 {
-    int i = 0;
+    register int i = 0;
     while (i < url->parts_len) {
-        free(url->parts[i]);
-        i++;
+        free(url->parts[i++]);
+    }
+
+    if (url->parts) {
+        free(url->parts);
     }
-    free(url->parts);
+
     url->parts = NULL;
     url->parts_len = 0;
 }

c/utils.c

@@ -76,7 +76,7 @@ const char *rpd_splitbyc(char **dest1, char **dest2, const char *src, const char
                 return NULL;
             }
             memcpy(*dest2, end, len);
-            (*dest2)[len + 1] = '\0';
+            (*dest2)[len] = '\0';
             return 0;
         }
         end++;

config.mk

@@ -1,4 +1,4 @@
-VERSION=0.4.1
+VERSION=0.4.2
 #arg Installation prefix
 PREFIX=/usr/local
 

include/keyval.h

@@ -44,6 +44,16 @@ typedef struct {
  */
 int rpd_keyval_init(rpd_keyval *keyval, int capacity);
 
+/*!
+ * \brief Reallocate key-value storage with new capacity.
+ *
+ * \param keyval Key-value storage.
+ * \param capacity New capacity.
+ *
+ * \return Status code.
+ */
+int rpd_keyval_realloc(rpd_keyval *keyval, int capacity);
+
 /*!
  * \brief Insert new key-value pair into the storage.
  *
@@ -87,7 +97,7 @@ rpd_keyval_item *rpd_keyval_find(const rpd_keyval *keyval, const char *key);
 rpd_keyval_item **rpd_keyval_findall(const rpd_keyval *keyval, const char *key);
 
 /*!
- * \brief Free key-val pairs and reset.
+ * \brief Cleanup key-val pairs and reset.
  *
  * This function don't will free rpd_keyval::items memory,
  * so capacity will be saved.
@@ -101,6 +111,16 @@ rpd_keyval_item **rpd_keyval_findall(const rpd_keyval *keyval, const char *key);
  */
 void rpd_keyval_cleanup(rpd_keyval *keyval);
 
+/*!
+ * \brief Free key-val pairs and reset.
+ *
+ * This function is the same as rpd_keyval_cleanup, but it's
+ * also free the rpd_keyval::items memory, so capacity after that is 0.
+ *
+ * \param keyval Key-value storage.
+ */
+void rpd_keyval_free(rpd_keyval *keyval);
+
 #ifdef __cplusplus
 }
 #endif

include/request.h

@@ -44,7 +44,14 @@ typedef struct {
 } rpd_req;
 
 /*!
- * \brief Parser string request method to enumeration value.
+ * \brief Fill request structure with initial values.
+ *
+ * \param dest Structure to fill.
+ */
+void rpd_req_init(rpd_req *dest);
+
+/*!
+ * \brief Parse string request method to enumeration value.
  *
  * \param method Request method string.
  *
@@ -53,14 +60,19 @@ typedef struct {
 enum rpd_req_methods rpd_req_smethod(const char *method);
 
 /*!
- * \brief Cleans request data.
- *
- * Request instance will not be freed, only it's data.
+ * \brief Clean request data.
  *
  * \param req Request instance.
  */
 void rpd_req_cleanup(rpd_req *req);
 
+/*!
+ * \brief Free request data memory.
+ *
+ * \param req Request instance.
+ */
+void rpd_req_free(rpd_req *req);
+
 #ifdef __cplusplus
 }
 #endif

include/response.h

@@ -118,15 +118,19 @@ int rpd_res_headers_str(char **dest, const rpd_res *src);
 int rpd_res_str(char **dest, const rpd_res *src);
 
 /*!
- * \brief Cleans response instance.
+ * \brief Clean response data.
  *
- * This function will not frees response instance,
- * only it's data.
- *
- * \param res Response instance to cleanup.
+ * \param res Response instance.
  */
 void rpd_res_cleanup(rpd_res *res);
 
+/*!
+ * \brief Free response data memory.
+ *
+ * \param res Response instance.
+ */
+void rpd_res_free(rpd_res *res);
+
 #ifdef __cplusplus
 }
 #endif

servers/fcgi.c

@@ -4,6 +4,7 @@
 #include "../include/servers/fcgi.h"
 #include "../c/utils.h"
 #include <ctype.h>
+#include <fcgiapp.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -81,6 +82,8 @@ int rpd_fcgi_server_start(rpd_app *app, const char *sock_path)
     listen_requests((void *) &data);
 #endif
 
+    FCGX_ShutdownPending();
+
     return 0;
 }
 
@@ -131,14 +134,16 @@ static void handle_request(rpd_app *app, FCGX_Request *fcgx_req)
     req = (rpd_req *) malloc(sizeof(rpd_req));
     res = (rpd_res *) malloc(sizeof(rpd_res));
 
-    fcgx_to_rpd_req(req, fcgx_req);
+    rpd_req_init(req);
     rpd_res_init(res);
 
+    fcgx_to_rpd_req(req, fcgx_req);
+
     rpd_app_handle_request(app, req, res);
     send_response(res, fcgx_req->out);
 
-    rpd_req_cleanup(req);
-    rpd_res_cleanup(res);
+    rpd_req_free(req);
+    rpd_res_free(res);
     free(req);
     free(res);
 }
@@ -163,8 +168,6 @@ static int fcgx_to_rpd_req(rpd_req *dest, FCGX_Request *req)
     rpd_url_parse(&dest->path, FCGX_GetParam("DOCUMENT_URI", req->envp));
     // dest->auth = FCGX_GetParam("HTTP_AUTHORIZATION", req->envp);
     // dest->cookie = FCGX_GetParam("HTTP_COOKIE", req->envp);
-    rpd_keyval_init(&dest->params, 0);
-    rpd_keyval_init(&dest->headers, 0);
     dest->headers.unique = 0;
 
     char **env = req->envp;
@@ -210,15 +213,17 @@ static int fcgx_to_rpd_req(rpd_req *dest, FCGX_Request *req)
 static int read_fcgx_req_body(char **dest, FCGX_Request *req)
 {
     char *clen = FCGX_GetParam("CONTENT_LENGTH", req->envp);
-    if (!clen)
+    if (!clen) {
         return 1;
+    }
+
     size_t len = atoll(clen);
 
     *dest = (char *) malloc(sizeof(char) * (len + 1));
     if (!*dest)
         return 2;
 
-    *dest[len] = '\0';
+    (*dest)[len] = '\0';
     FCGX_GetStr(*dest, len, req->in);
 
     return 0;
@@ -226,8 +231,6 @@ static int read_fcgx_req_body(char **dest, FCGX_Request *req)
 
 static int env_to_req_header(char **key, char **val, const char *env)
 {
-    const char *ptr = NULL;
-
     rpd_splitbyc(key, val, env, '=');
     if (!*key || !*val) {
         if (*key)