gistfile1.txt

diff --git a/include/entry.h b/include/entry.h
index e604586..79f9d1f 100644
--- a/include/entry.h
+++ b/include/entry.h
@@ -70,7 +70,7 @@ typedef struct xcb_xrm_component_t {
 /** Used in xcb_xrm_entry_parse. */
 typedef struct xcb_xrm_entry_parser_state_t {
     xcb_xrm_entry_parser_chunk_status_t chunk;
-    char buffer[4096];
+    char buffer[4096]; /* Let's overflow that! */
     char *buffer_pos;
     xcb_xrm_binding_type_t current_binding_type;
 } xcb_xrm_entry_parser_state_t;
diff --git a/include/match.h b/include/match.h
index 94984d0..cce4c83 100644
--- a/include/match.h
+++ b/include/match.h
@@ -55,7 +55,7 @@ typedef struct xcb_xrm_match_t {
 	xcb_xrm_entry_t *entry;
     /* An array where the n-th element describes how the n-th element of the
      * query strings was matched. */
-	int *flags;
+	xcb_xrm_match_flags_t *flags;
 } xcb_xrm_match_t;
 
 /**
diff --git a/include/util.h b/include/util.h
index 4aea580..6f1818b 100644
--- a/include/util.h
+++ b/include/util.h
@@ -39,10 +39,8 @@
 
 #define FREE(p)          \
     do {                 \
-        if (p != NULL) { \
-            free(p);     \
-            p = NULL;    \
-        }                \
+        free(p);         \
+        p = NULL;        \
     } while (0)
 
 #undef MAX
diff --git a/include/xcb_xrm.h b/include/xcb_xrm.h
index 0a27b5a..b31624c 100644
--- a/include/xcb_xrm.h
+++ b/include/xcb_xrm.h
@@ -29,7 +29,8 @@
 #ifndef __XCB_XRM_H__
 #define __XCB_XRM_H__
 
-#include "externals.h"
+#include <stdbool.h>
+#include <xcb/xcb.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -55,6 +56,11 @@ extern "C" {
  * xcb_screen_t *screen = xcb_aux_get_screen(conn, screennr);
  *
  * xcb_xrm_database_t *database = xcb_xrm_database_from_resource_manager(conn, screen);
+ * FIXME: There should be a function like XGetDefault():
+ * - Try to get the DB from the server.
+ * - If nothing was found, check ~/.Xdefaults
+ * - For some reason this then gets the XENVIRONMENT env var (defaulting to
+ *   ~/.Xdefaults-) and merges the two databases
  *
  * xcb_xrm_resource_t *resource;
  * if (xcb_xrm_resource_get(database, "Xft.dpi", "Xft.dpi", &resource) < 0) {
@@ -98,8 +104,9 @@ typedef struct xcb_xrm_resource_t xcb_xrm_resource_t;
 
 /**
  * Loads the RESOURCE_MANAGER property and creates a database with its
- * contents. If the database could not be created, thie function will return
+ * contents. If the database could not be created, this function will return
  * NULL.
+ * This function handles #include-directives (TODO: Should it?).
  *
  * @param conn A working XCB connection.
  * @param screen The xcb_screen_t* screen to use.
@@ -112,6 +119,7 @@ xcb_xrm_database_t *xcb_xrm_database_from_resource_manager(xcb_connection_t *con
 /**
  * Creates a database from the given string.
  * If the database could not be created, this function will return NULL.
+ * This function handles #include-directives.
  *
  * @param str The resource string.
  * @returns The database described by the resource string.
@@ -123,6 +131,7 @@ xcb_xrm_database_t *xcb_xrm_database_from_string(const char *str);
 /**
  * Creates a database from a given file.
  * If the file cannot be found or opened, NULL is returned.
+ * This function handles #include-directives.
  *
  * @param filename Valid filename.
  * @returns The database described by the file's contents.
@@ -145,6 +154,8 @@ char *xcb_xrm_database_to_string(xcb_xrm_database_t *database);
  * overridden if override is set; otherwise, the value is discarded.
  * The source database will implicitly be free'd and must not be used
  * afterwards.
+ *  That's a pity. Wouldn't it be more useful to keep the source_db alive? That
+ *  might make it easier to track the life time of databases.
  * If NULL is passed for target_db, a new and empty database will be created
  * and returned in the pointer.
  *
@@ -202,8 +213,9 @@ void xcb_xrm_database_free(xcb_xrm_database_t *database);
  * components as the resource name.
  * @param resource A pointer to a xcb_xrm_resource_t* which will be modified to
  * contain the matched resource. Note that this resource must be free'd by the
- * caller.
+ * caller via a call to @ref xcb_xrm_resource_free ().
  * @return 0 on success, a negative error code otherwise.
+ *  What's the meaning of "error code"?
  */
 int xcb_xrm_resource_get(xcb_xrm_database_t *database, const char *res_name, const char *res_class,
                          xcb_xrm_resource_t **resource);
@@ -211,6 +223,9 @@ int xcb_xrm_resource_get(xcb_xrm_database_t *database, const char *res_name, con
 /**
  * Returns the string value of the resource.
  * The string is owned by the caller and must be free'd.
+ * Why the copy? Wouldn't it be easier to return a const char that is valid as
+ * long as the xcb_xrm_resource_t and callers who want a copy can strdup()
+ * themselves?
  *
  * @param resource The resource to use.
  * @returns The string value of the given resource.
diff --git a/src/database.c b/src/database.c
index cba96cf..d5cec58 100644
--- a/src/database.c
+++ b/src/database.c
@@ -37,7 +37,7 @@ void xcb_xrm_database_put(xcb_xrm_database_t *database, xcb_xrm_entry_t *entry,
 
 /*
  * Loads the RESOURCE_MANAGER property and creates a database with its
- * contents. If the database could not be created, thie function will return
+ * contents. If the database could not be created, this function will return
  * NULL.
  *
  * @param conn A working XCB connection.
@@ -133,23 +133,20 @@ xcb_xrm_database_t *xcb_xrm_database_from_string(const char *_str) {
                 char *filename;
                 int j = strlen(line) - 1;
 
-                /* Skip whitespace */
+                /* Skip whitespace and any number of quotes */
                 while (line[i] == ' ' || line[i] == '\t' || line[i] == '"')
                     i++;
                 while (line[j] == ' ' || line[j] == '\t' || line[j] == '"')
                     j--;
 
-                filename = scalloc(1, j - i + 2);
-                if (filename == NULL)
+                if (j < i)
+                    /* Only whitespace left in the line */
                     continue;
 
-                memcpy(filename, &line[i], j - i + 1);
-                filename[j - 1 + 1] = '\0';
-
-                // TODO XXX Filename globbing
+                filename = &line[i];
+                line[j+1] = '\0';
 
                 included = xcb_xrm_database_from_file(filename);
-                FREE(filename);
 
                 if (included != NULL)
                     xcb_xrm_database_combine(included, &database, true);
@@ -199,7 +196,11 @@ char *xcb_xrm_database_to_string(xcb_xrm_database_t *database) {
     TAILQ_FOREACH(entry, database, entries) {
         char *entry_str = xcb_xrm_entry_to_string(entry);
         char *tmp;
-        sasprintf(&tmp, "%s%s\n", result == NULL ? "" : result, entry_str);
+        if (sasprintf(&tmp, "%s%s\n", result == NULL ? "" : result, entry_str) < 0) {
+            FREE(entry_str);
+            FREE(result);
+            return NULL;
+        }
         FREE(entry_str);
         FREE(result);
         result = tmp;
@@ -264,7 +265,12 @@ void xcb_xrm_database_put_resource(xcb_xrm_database_t **database, const char *re
         *database = xcb_xrm_database_from_string("");
 
     escaped = xcb_xrm_entry_escape_value(value);
-    sasprintf(&line, "%s: %s", resource, escaped);
+    if (escaped == NULL)
+        return;
+    if (sasprintf(&line, "%s: %s", resource, escaped) < 0) {
+        FREE(escaped);
+        return;
+    }
     FREE(escaped);
     xcb_xrm_database_put_resource_line(database, line);
     FREE(line);
@@ -295,7 +301,7 @@ void xcb_xrm_database_put_resource_line(xcb_xrm_database_t **database, const cha
     if (xcb_xrm_entry_parse(line, &entry, false) == 0) {
         xcb_xrm_database_put(*database, entry, true);
     } else {
-        xcb_xrm_entry_free(entry);
+        xcb_xrm_entry_free(entry); /* XXX: This is a double-free, I think */
     }
 }
 
@@ -322,7 +328,7 @@ void xcb_xrm_database_free(xcb_xrm_database_t *database) {
 void xcb_xrm_database_put(xcb_xrm_database_t *database, xcb_xrm_entry_t *entry, bool override) {
     xcb_xrm_entry_t *current;
 
-    if (entry == NULL)
+    if (database == NULL || entry == NULL)
         return;
 
     /* Let's see whether this is a duplicate entry. */
diff --git a/src/entry.c b/src/entry.c
index 2e37063..6b86b05 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -102,7 +102,7 @@ int xcb_xrm_entry_parse(const char *_str, xcb_xrm_entry_t **_entry, bool resourc
     char *str;
     xcb_xrm_entry_t *entry = NULL;
     xcb_xrm_component_t *last;
-    char value_buf[4096];
+    char value_buf[4096]; /* Overflow me */
     char *value_pos = value_buf;
 
     xcb_xrm_entry_parser_state_t state = {
@@ -164,6 +164,9 @@ int xcb_xrm_entry_parse(const char *_str, xcb_xrm_entry_t **_entry, bool resourc
                 goto process_normally;
             case ':':
                 if (resource_only) {
+                    /* This suggests that this should be split into two parsers,
+                     * one resource_only, one for the value
+                     */
                     goto done_error;
                 }
 
@@ -323,11 +326,14 @@ char *xcb_xrm_entry_to_string(xcb_xrm_entry_t *entry) {
     assert(entry != NULL);
     TAILQ_FOREACH(component, &(entry->components), components) {
         char *tmp;
-        sasprintf(&tmp, "%s%s%s", result == NULL ? "" : result,
+        if (sasprintf(&tmp, "%s%s%s", result == NULL ? "" : result,
                 (is_first && component->binding_type == BT_TIGHT)
                     ? ""
                     : (component->binding_type == BT_TIGHT ? "." : "*"),
-                component->type == CT_NORMAL ? component->name : "?");
+                component->type == CT_NORMAL ? component->name : "?") < 0) {
+            FREE(result);
+            return NULL;
+        }
         FREE(result);
         result = tmp;
 
@@ -335,7 +341,11 @@ char *xcb_xrm_entry_to_string(xcb_xrm_entry_t *entry) {
     }
 
     escaped_value = xcb_xrm_entry_escape_value(entry->value);
-    sasprintf(&value_buf, "%s: %s", result, escaped_value);
+    if (sasprintf(&value_buf, "%s: %s", result, escaped_value) < 0) {
+        FREE(escaped_value);
+        FREE(result);
+        return NULL;
+    }
     FREE(escaped_value);
     FREE(result);
     result = value_buf;
@@ -348,33 +358,26 @@ char *xcb_xrm_entry_to_string(xcb_xrm_entry_t *entry) {
  *
  */
 char *xcb_xrm_entry_escape_value(const char *value) {
-    char *copy;
     char *escaped;
     char *outwalk;
     int new_size = strlen(value) + 1;
 
-    copy = sstrdup(value);
-    if (copy == NULL)
-        return NULL;
-
-    if (copy[0] == ' ' || copy[0] == '\t')
+    if (value[0] == ' ' || value[0] == '\t')
         new_size++;
-    for (char *walk = copy; *walk != '\0'; walk++) {
+    for (const char *walk = value; *walk != '\0'; walk++) {
         if (*walk == '\n' || *walk == '\\')
             new_size++;
     }
 
     escaped = scalloc(1, new_size);
-    if (escaped == NULL) {
-        FREE(copy);
+    if (escaped == NULL)
         return NULL;
-    }
 
     outwalk = escaped;
-    if (copy[0] == ' ' || copy[0] == '\t') {
+    if (value[0] == ' ' || value[0] == '\t') {
         *(outwalk++) = '\\';
     }
-    for (char *walk = copy; *walk != '\0'; walk++) {
+    for (const char *walk = value; *walk != '\0'; walk++) {
         if (*walk == '\n') {
             *(outwalk++) = '\\';
             *(outwalk++) = 'n';
@@ -386,7 +389,6 @@ char *xcb_xrm_entry_escape_value(const char *value) {
         }
     }
     *outwalk = '\0';
-    FREE(copy);
 
     return escaped;
 }
diff --git a/src/match.c b/src/match.c
index 307daf3..cbf618b 100644
--- a/src/match.c
+++ b/src/match.c
@@ -79,8 +79,10 @@ int xcb_xrm_match(xcb_xrm_database_t *database, xcb_xrm_entry_t *query_name, xcb
 
     if (best_match != NULL) {
         resource->value = sstrdup(best_match->entry->value);
-        if (resource->value == NULL)
+        if (resource->value == NULL) {
+            __match_free(best_match);
             return -FAILURE;
+        }
 
         __match_free(best_match);
         return SUCCESS;
@@ -149,6 +151,7 @@ static int __match_matches(xcb_xrm_entry_t *db_entry, xcb_xrm_entry_t *query_nam
                 break;
             default:
                 /* Never reached. */
+                assert(0);
                 return -FAILURE;
         }
     }
diff --git a/src/util.c b/src/util.c
index 6fdc50a..f6f0f89 100644
--- a/src/util.c
+++ b/src/util.c
@@ -32,10 +32,12 @@
 
 char *sstrdup(const char *str) {
     return strdup(str);
+    /* Remove, it's unnecessary since err() isn't used any more */
 }
 
 void *scalloc(size_t num, size_t size) {
     return calloc(num, size);
+    /* Remove */
 }
 
 int sasprintf(char **strp, const char *fmt, ...) {
@@ -45,6 +47,7 @@ int sasprintf(char **strp, const char *fmt, ...) {
     va_start(args, fmt);
     result = vasprintf(strp, fmt, args);
     va_end(args);
+    /* Remove */
     return result;
 }
 
@@ -85,7 +88,7 @@ char *file_get_contents(const char *filename) {
     file_size = stbuf.st_size;
 
     /* Read the file content. */
-    content = scalloc(file_size, 1);
+    content = scalloc(file_size + 1, 1);
     if (content == NULL) {
         fclose(file);
         return NULL;
diff --git a/tests/test.c b/tests/test.c
index f7b9d17..53d95c9 100644
--- a/tests/test.c
+++ b/tests/test.c
@@ -416,7 +416,7 @@ static void setup(void) {
 }
 
 static void cleanup(void) {
-    xcb_disconnect(conn);
+    XCloseDisplay(display);
 }
 
 static int check_parse_entry(const char *str, const char *value, const char *bindings, const int count, ...) {