Fix a few more memory bugs. Still have some to tackle.

TODO.txt

@@ -52,7 +52,7 @@ Phase 2: Building a foundation
 	[~] Data abstraction layer
 		[ ] Database upgrades/migration path
 		[ ] Caching and cache control
-		[ ] Make memory info access O(1)
+		[x] Make memory info access O(1)
 	[x] Error generation
 [x] Properly implement the command line options as stated in telodendria(8)
 [x] Remove "chroot" option, just chroot into the data dir, and make the log
@@ -61,6 +61,7 @@ Phase 2: Building a foundation
 [x] Fix memory leaks
 [ ] Fix bug where the socket stays open after quit.
 [ ] Figure out how to write unit tests for array/hashmap/etc
+[ ] Add recipe to td script to upload patches to the Matrix room
 
 Phase 3: Welcome to Matrix
 
@@ -135,3 +136,4 @@ Documentation
 [x] Add onboarding documentation
 	[x] Building from source
 	[!] Writing config file (see Config.5)
+

src/HttpServer.c

@@ -122,17 +122,37 @@ HttpServerContextFree(HttpServerContext * c)
 
     while (HashMapIterate(c->requestHeaders, &key, &val))
     {
-        /* Since we create these, we know they're always on the heap */
+        /*
+         * These are always parsed from the request, so they should
+         * always be on the heap.
+         */
         Free(key);
         Free(val);
     }
     HashMapFree(c->requestHeaders);
 
     while (HashMapIterate(c->responseHeaders, &key, &val))
+    {
+        /*
+         * These are generated by code. As such, they may be either
+         * on the heap, or on the stack, depending on how they were
+         * added.
+         *
+         * Basically, if the memory API knows about a pointer, then
+         * it can be freed. If it doesn't know about a pointer, skip
+         * freeing it because it's probably a stack pointer.
+         */
+
+        if (MemoryInfoGet(key))
         {
             Free(key);
+        }
+
+        if (MemoryInfoGet(val))
+        {
             Free(val);
         }
+    }
 
     HashMapFree(c->responseHeaders);
 
@@ -432,7 +452,7 @@ HttpServerWorkerThread(void *args)
         }
 
         /* Get the first line of the request */
-        lineLen = getline(&line, &lineSize, fp);
+        lineLen = UtilGetLine(&line, &lineSize, fp);
         if (lineLen == -1)
         {
             goto bad_request;
@@ -501,7 +521,7 @@ HttpServerWorkerThread(void *args)
             goto internal_error;
         }
 
-        while ((lineLen = getline(&line, &lineSize, fp)) != -1)
+        while ((lineLen = UtilGetLine(&line, &lineSize, fp)) != -1)
         {
             char *headerKey;
             char *headerValue;

src/Memory.c

@@ -33,16 +33,105 @@ struct MemoryInfo
     const char *file;
     int line;
     void *pointer;
-
-    MemoryInfo *next;
-    MemoryInfo *prev;
 };
 
 static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
-static MemoryInfo *lastAllocation = NULL;
 static void (*hook) (MemoryAction, MemoryInfo *, void *) = NULL;
 static void *hookArgs = NULL;
 
+static MemoryInfo **allocations = NULL;
+static size_t allocationsSize = 0;
+static size_t allocationsLen = 0;
+
+static size_t
+MemoryHash(void *p)
+{
+    return (((size_t) p) >> 2 * 7) % allocationsSize;
+}
+
+static int
+MemoryInsert(MemoryInfo * a)
+{
+    size_t hash;
+
+    if (!allocations)
+    {
+        allocationsSize = 64;
+        allocations = calloc(allocationsSize, sizeof(void *));
+        if (!allocations)
+        {
+            return 0;
+        }
+    }
+
+    if ((allocationsLen + 1) >= (0.75 * allocationsSize))
+    {
+        size_t i;
+        size_t tmpAllocationsSize = allocationsSize;
+        MemoryInfo **tmpAllocations;
+
+        allocationsSize *= 2;
+        tmpAllocations = calloc(allocationsSize, sizeof(void *));
+
+        if (!tmpAllocations)
+        {
+            return 0;
+        }
+
+        for (i = 0; i < tmpAllocationsSize; i++)
+        {
+            if (allocations[i])
+            {
+                hash = MemoryHash(allocations[i]->pointer);
+
+                while (tmpAllocations[hash])
+                {
+                    hash = (hash + 1) % allocationsSize;
+                }
+
+                tmpAllocations[hash] = allocations[i];
+            }
+        }
+
+        free(allocations);
+        allocations = tmpAllocations;
+    }
+
+    hash = MemoryHash(a->pointer);
+
+    while (allocations[hash])
+    {
+        hash = (hash + 1) % allocationsSize;
+    }
+
+    allocations[hash] = a;
+    allocationsLen++;
+
+    return 1;
+}
+
+static void
+MemoryDelete(MemoryInfo * a)
+{
+    size_t hash = MemoryHash(a->pointer);
+    size_t count = 0;
+
+    while (count <= allocationsSize)
+    {
+        if (allocations[hash] && allocations[hash] == a)
+        {
+            allocations[hash] = NULL;
+            allocationsLen--;
+            return;
+        }
+        else
+        {
+            hash = (hash + 1) % allocationsSize;
+            count++;
+        }
+    }
+}
+
 void *
 MemoryAllocate(size_t size, const char *file, int line)
 {
@@ -70,16 +159,15 @@ MemoryAllocate(size_t size, const char *file, int line)
     a->file = file;
     a->line = line;
     a->pointer = p;
-    a->next = NULL;
-    a->prev = lastAllocation;
 
-    if (lastAllocation)
+    if (!MemoryInsert(a))
     {
-        lastAllocation->next = a;
+        free(a);
+        free(p);
+        pthread_mutex_unlock(&lock);
+        return NULL;
     }
 
-    lastAllocation = a;
-
     if (hook)
     {
         hook(MEMORY_ALLOCATE, a, hookArgs);
@@ -100,99 +188,102 @@ MemoryReallocate(void *p, size_t size, const char *file, int line)
         return MemoryAllocate(size, file, line);
     }
 
+    a = MemoryInfoGet(p);
+    if (a)
+    {
         pthread_mutex_lock(&lock);
-
-    a = lastAllocation;
-    while (a)
-    {
-        if (a->pointer == p)
-        {
-            new = realloc(p, size);
+        new = realloc(a->pointer, size);
         if (new)
         {
-                a->pointer = new;
+            MemoryDelete(a);
             a->size = size;
             a->file = file;
             a->line = line;
 
+            a->pointer = new;
+            MemoryInsert(a);
+
             if (hook)
             {
                 hook(MEMORY_REALLOCATE, a, hookArgs);
             }
-            }
 
-            break;
         }
-
-        a = a->prev;
-    }
-
         pthread_mutex_unlock(&lock);
+    }
+    else if (hook)
+    {
+        a = malloc(sizeof(MemoryInfo));
+        if (a)
+        {
+            a->size = 0;
+            a->file = file;
+            a->line = line;
+            a->pointer = p;
+            hook(MEMORY_BAD_POINTER, a, hookArgs);
+            free(a);
+        }
+    }
+
 
     return new;
 }
 
 void
-MemoryFree(void *p)
+MemoryFree(void *p, const char *file, int line)
 {
     MemoryInfo *a;
 
+    if (!p)
+    {
+        return;
+    }
+
+    a = MemoryInfoGet(p);
+    if (a)
+    {
         pthread_mutex_lock(&lock);
-
-    a = lastAllocation;
-
-    while (a)
-    {
-        if (a->pointer == p)
-        {
-            if (a->prev)
-            {
-                a->prev->next = a->next;
-            }
-            else
-            {
-                lastAllocation = a->next;
-            }
-
-            if (a->next)
-            {
-                a->next->prev = a->prev;
-            }
-            else
-            {
-                lastAllocation = a->prev;
-            }
-
         if (hook)
         {
+            a->file = file;
+            a->line = line;
             hook(MEMORY_FREE, a, hookArgs);
         }
-
+        MemoryDelete(a);
+        free(a->pointer);
         free(a);
-            free(p);
-
-            break;
-        }
-
-        a = a->prev;
-    }
 
         pthread_mutex_unlock(&lock);
+    }
+    else if (hook)
+    {
+        a = malloc(sizeof(MemoryInfo));
+        if (a)
+        {
+            a->file = file;
+            a->line = line;
+            a->size = 0;
+            a->pointer = p;
+            hook(MEMORY_BAD_POINTER, a, hookArgs);
+            free(a);
+        }
+    }
 }
 
 size_t
 MemoryAllocated(void)
 {
-    MemoryInfo *a;
+    size_t i;
     size_t total = 0;
 
     pthread_mutex_lock(&lock);
 
-    a = lastAllocation;
-    while (a)
+    for (i = 0; i < allocationsSize; i++)
     {
-        total += a->size;
-        a = a->prev;
+        if (allocations[i])
+        {
+            total += allocations[i]->size;
+        }
     }
 
     pthread_mutex_unlock(&lock);
@@ -203,22 +294,23 @@ MemoryAllocated(void)
 void
 MemoryFreeAll(void)
 {
-    MemoryInfo *a;
+    size_t i;
 
     pthread_mutex_lock(&lock);
 
-    a = lastAllocation;
-    while (a)
+    for (i = 0; i < allocationsSize; i++)
     {
-        MemoryInfo *prev = a->prev;
-
-        free(a->pointer);
-        free(a);
-
-        a = prev;
+        if (allocations[i])
+        {
+            free(allocations[i]->pointer);
+            free(allocations[i]);
+        }
     }
 
-    lastAllocation = NULL;
+    free(allocations);
+    allocations = NULL;
+    allocationsSize = 0;
+    allocationsLen = 0;
 
     pthread_mutex_unlock(&lock);
 }
@@ -226,22 +318,28 @@ MemoryFreeAll(void)
 MemoryInfo *
 MemoryInfoGet(void *p)
 {
-    MemoryInfo *a;
+    size_t hash, count;
 
     pthread_mutex_lock(&lock);
 
-    a = lastAllocation;
-    while (a)
+    hash = MemoryHash(p);
+
+    count = 0;
+    while (count <= allocationsSize)
     {
-        if (a->pointer == p)
+        if (!allocations[hash] || allocations[hash]->pointer != p)
+        {
+            hash = (hash + 1) % allocationsSize;
+            count++;
+        }
+        else
         {
             break;
         }
-
-        a = a->prev;
     }
 
-    return a;
+    pthread_mutex_unlock(&lock);
+    return allocations[hash];
 }
 
 size_t
@@ -291,15 +389,16 @@ MemoryInfoGetPointer(MemoryInfo * a)
 void
  MemoryIterate(void (*iterFunc) (MemoryInfo *, void *), void *args)
 {
-    MemoryInfo *a;
+    size_t i;
 
     pthread_mutex_lock(&lock);
 
-    a = lastAllocation;
-    while (a)
+    for (i = 0; i < allocationsSize; i++)
     {
-        iterFunc(a, args);
-        a = a->prev;
+        if (allocations[i])
+        {
+            iterFunc(allocations[i], args);
+        }
     }
 
     pthread_mutex_unlock(&lock);

src/Telodendria.c

@@ -58,8 +58,8 @@ TelodendriaMemoryHook(MemoryAction a, MemoryInfo * i, void *args)
             action = "Freed";
             break;
         case MEMORY_BAD_POINTER:
-            Log(lc, LOG_WARNING, "Bad pointer passed into a memory function: %p", (void *) i);
-            return;
+            action = "Bad pointer to";
+            break;
         default:
             action = "Unknown operation on";
             break;
@@ -154,6 +154,7 @@ main(int argc, char **argv)
     memset(&matrixArgs, 0, sizeof(matrixArgs));
 
     lc = LogConfigCreate();
+    LogConfigLevelSet(lc, LOG_DEBUG);
 
     if (!lc)
     {

src/Util.c

@@ -25,12 +25,14 @@
 
 #include <Memory.h>
 
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
 #include <math.h>
 #include <time.h>
 #include <sys/time.h>
+#include <errno.h>
 
 unsigned long
 UtilServerTs(void)
@@ -174,3 +176,86 @@ UtilStringToBytes(char *str)
 
     return bytes;
 }
+
+ssize_t
+UtilGetDelim(char **linePtr, size_t * n, int delim, FILE * stream)
+{
+    char *curPos, *newLinePtr;
+    size_t newLinePtrLen;
+    int c;
+
+    if (!linePtr || !n || !stream)
+    {
+        errno = EINVAL;
+        return -1;
+    }
+
+    if (*linePtr == NULL)
+    {
+        *n = 128;
+
+        if (!(*linePtr = Malloc(*n)))
+        {
+            errno = ENOMEM;
+            return -1;
+        }
+    }
+
+    curPos = *linePtr;
+
+    while (1)
+    {
+        c = getc(stream);
+
+        if (ferror(stream) || (c == EOF && curPos == *linePtr))
+        {
+            return -1;
+        }
+
+        if (c == EOF)
+        {
+            break;
+        }
+
+        if ((*linePtr + *n - curPos) < 2)
+        {
+            if (SSIZE_MAX / 2 < *n)
+            {
+#ifdef EOVERFLOW
+                errno = EOVERFLOW;
+#else
+                errno = ERANGE;
+#endif
+                return -1;
+            }
+
+            newLinePtrLen = *n * 2;
+
+            if (!(newLinePtr = Realloc(*linePtr, newLinePtrLen)))
+            {
+                errno = ENOMEM;
+                return -1;
+            }
+
+            curPos = newLinePtr + (curPos - *linePtr);
+            *linePtr = newLinePtr;
+            *n = newLinePtrLen;
+        }
+
+        *curPos++ = (char) c;
+
+        if (c == delim)
+        {
+            break;
+        }
+    }
+
+    *curPos = '\0';
+    return (ssize_t) (curPos - *linePtr);
+}
+
+ssize_t
+UtilGetLine(char **linePtr, size_t * n, FILE * stream)
+{
+    return UtilGetDelim(linePtr, n, '\n', stream);
+}

src/include/Memory.h

@@ -36,13 +36,13 @@ typedef enum MemoryAction
 
 #define Malloc(x) MemoryAllocate(x, __FILE__, __LINE__)
 #define Realloc(x, s) MemoryReallocate(x, s, __FILE__, __LINE__)
-#define Free(x) MemoryFree(x)
+#define Free(x) MemoryFree(x, __FILE__, __LINE__)
 
 typedef struct MemoryInfo MemoryInfo;
 
 extern void *MemoryAllocate(size_t, const char *, int);
 extern void *MemoryReallocate(void *, size_t, const char *, int);
-extern void MemoryFree(void *);
+extern void MemoryFree(void *, const char *, int);
 
 extern size_t MemoryAllocated(void);
 extern void MemoryFreeAll(void);

src/include/Util.h

@@ -31,7 +31,9 @@
 #ifndef TELODENDRIA_UTIL_H
 #define TELODENDRIA_UTIL_H
 
+#include <stdio.h>
 #include <stddef.h>
+#include <sys/types.h>
 
 /*
  * Get the current type in milliseconds since the Unix epoch. This uses
@@ -101,4 +103,10 @@ extern int
 extern size_t
  UtilStringToBytes(char *);
 
+extern ssize_t
+ UtilGetDelim(char **, size_t *, int, FILE *);
+
+extern ssize_t
+ UtilGetLine(char **, size_t *, FILE *);
+
 #endif                             /* TELODENDRIA_UTIL_H */