Rework message parsing to use getline rather than mmap.
authorCarl Worth <cworth@cworth.org>
Mon, 19 Oct 2009 23:38:44 +0000 (16:38 -0700)
committerCarl Worth <cworth@cworth.org>
Mon, 19 Oct 2009 23:38:44 +0000 (16:38 -0700)
The line-based parsing can be a bit awkward when wanting to peek
ahead, (say, for folded header values), but it's so convenient
to be able to trust that a string terminator exists on every
line so it cleans up the code considerably.

message.c
notmuch-private.h

index 97df4b270fefe344577ae32f24c7bc51331a0499..03583c8d0e43f122215bdbcdd6e7c9ca92242a79 100644 (file)
--- a/message.c
+++ b/message.c
 
 #include <glib.h> /* GHashTable */
 
+typedef struct {
+    char *str;
+    size_t size;
+    size_t len;
+} header_value_closure_t;
+
 struct _notmuch_message {
-    /* File objects */
-    int fd;
-    void *map;
+    /* File object */
+    FILE *file;
 
     /* Header storage */
     int restrict_headers;
     GHashTable *headers;
 
     /* Parsing state */
-    char *start;
-    size_t size;
-    const char *next_line;
+    char *line;
+    size_t line_size;
+    header_value_closure_t value;
+
     int parsing_started;
     int parsing_finished;
 };
@@ -66,20 +72,11 @@ notmuch_message_t *
 notmuch_message_open (const char *filename)
 {
     notmuch_message_t *message;
-    struct stat st;
 
     message = xcalloc (1, sizeof (notmuch_message_t));
 
-    message->fd = open (filename, O_RDONLY);
-    if (message->fd < 0)
-       goto FAIL;
-
-    if (fstat (message->fd, &st) < 0)
-       goto FAIL;
-
-    message->map = mmap (NULL, st.st_size, PROT_READ, MAP_PRIVATE,
-                       message->fd, 0);
-    if (message->map == MAP_FAILED)
+    message->file = fopen (filename, "r");
+    if (message->file == NULL)
        goto FAIL;
 
     message->headers = g_hash_table_new_full (strcase_hash,
@@ -87,9 +84,6 @@ notmuch_message_open (const char *filename)
                                              free,
                                              free);
 
-    message->start = (char *) message->map;
-    message->size = st.st_size;
-    message->next_line = message->start;
     message->parsing_started = 0;
     message->parsing_finished = 0;
 
@@ -111,10 +105,8 @@ notmuch_message_close (notmuch_message_t *message)
     if (message->headers)
        g_hash_table_unref (message->headers);
 
-    if (message->map)
-       munmap (message->map, message->size);
-    if (message->fd)
-       close (message->fd);
+    if (message->file)
+       fclose (message->file);
 
     free (message);
 }
@@ -151,80 +143,44 @@ notmuch_message_restrict_headers (notmuch_message_t *message, ...)
     notmuch_message_restrict_headersv (message, va_headers);
 }
 
-/* With our mmapped file, we don't get the benefit of terminated
- * strings, so we can't use things like strchr(). We don't even know
- * if there's a newline at the end of the file so we also have to be
- * careful of that. Basically, every time we advance a pointer while
- * parsing we must ensure we don't go beyond our buffer.
- */
-#define WITHIN(s) (((s) - message->start) < (message->size -1))
-
-/* In each of the macros below, "without overrunning the buffer" means
- * that the macro will never dereference a character beyond the end of
- * the buffer. However, all of the macros may return a pointer
- * pointing to the first character beyond the buffer. So callers
- * should test with WITHIN before dereferencing the result. */
-
-/* Advance 'ptr' until pointing at a non-space character in the same
- * line, (without overrunning the buffer) */
-#define SKIP_SPACE_IN_LINE(ptr)                                      \
-    while (WITHIN (ptr) && (*(ptr) == ' ' || *(ptr) == '\t')) \
-       (ptr)++;
-
-/* Advance 'ptr' until pointing at a non-space character, (without
- * overrunning the buffer) */
-#define SKIP_SPACE(ptr)                                \
-    while (WITHIN (ptr) && isspace(*(ptr)))    \
-       (ptr)++;
-
-/* Advance 'ptr' to the first occurrence of 'c' within the same
- * line, (without overrunning the buffer). */
-#define ADVANCE_TO(ptr, c)                     \
-    while (WITHIN (ptr) && *(ptr) != '\n' &&    \
-          *(ptr) != (c))                       \
-    {                                          \
-       (ptr)++;                                \
+void
+copy_header_unfolding (header_value_closure_t *value,
+                      const char *chunk)
+{
+    char *last;
+
+    if (chunk == NULL)
+       return;
+
+    while (*chunk == ' ' || *chunk == '\t')
+       chunk++;
+
+    if (value->len + 1 + strlen (chunk) + 1 > value->size) {
+       int new_size = value->size;
+       if (value->size == 0)
+           new_size = strlen (chunk) + 1;
+       else
+           while (value->len + 1 + strlen (chunk) + 1 > new_size)
+               new_size *= 2;
+       value->str = xrealloc (value->str, new_size);
+       value->size = new_size;
     }
 
-/* Advance 'ptr' to the beginning of the next line not starting with
- * an initial tab character, (without overruning the buffer). */
-#define ADVANCE_TO_NEXT_HEADER_LINE(ptr)       \
-    do {                                       \
-       ADVANCE_TO ((ptr), '\n');               \
-       if (WITHIN (ptr))                       \
-           (ptr)++;                            \
-    } while (WITHIN (ptr) &&                   \
-            (*(ptr) == '\t' || *(ptr) == ' '));
-        
-char *
-copy_header_value (const char *start, const char *end)
-{
-    const char *s;
-    char *result, *r;
-    int was_newline = 0;
-
-    result = xmalloc (end - start + 1);
-
-    s = start;
-    r = result;
-
-    while (s < end) {
-       if (*s == '\n') {
-           was_newline = 1;
-       } else {
-           if (*s == '\t' && was_newline)
-               *r = ' ';
-           else
-               *r = *s;
-           r++;
-           was_newline = 0;
-       }
-       s++;
+    last = value->str + value->len;
+    if (value->len) {
+       *last = ' ';
+       last++;
+       value->len++;
     }
 
-    *r = '\0';
+    strcpy (last, chunk);
+    value->len += strlen (chunk);
 
-    return result;
+    last = value->str + value->len - 1;
+    if (*last == '\n') {
+       *last = '\0';
+       value->len--;
+    }
 }
 
 const char *
@@ -232,8 +188,8 @@ notmuch_message_get_header (notmuch_message_t *message,
                            const char *header_desired)
 {
     int contains;
-    const char *s, *colon;
     char *header, *value;
+    const char *s, *colon;
     int match;
 
     message->parsing_started = 1;
@@ -247,54 +203,82 @@ notmuch_message_get_header (notmuch_message_t *message,
     if (message->parsing_finished)
        return NULL;
 
-    while (1) {
-       s = message->next_line;
+#define NEXT_HEADER_LINE(closure)                              \
+    do {                                                       \
+       ssize_t bytes_read = getline (&message->line,           \
+                                     &message->line_size,      \
+                                     message->file);           \
+       if (bytes_read == -1) {                                 \
+           message->parsing_finished = 1;                      \
+           break;                                              \
+       }                                                       \
+       if (*message->line == '\n') {                           \
+           message->parsing_finished = 1;                      \
+           break;                                              \
+       }                                                       \
+       if (closure &&                                          \
+           (*message->line == ' ' || *message->line == '\t'))  \
+       {                                                       \
+           copy_header_unfolding ((closure), message->line);   \
+       }                                                       \
+    } while (*message->line == ' ' || *message->line == '\t');
+
+    if (message->line == NULL)
+       NEXT_HEADER_LINE (NULL);
 
-       if (*s == '\n') {
-           message->parsing_finished = 1;
-           return NULL;
-       }
+    while (1) {
 
-       if (*s == '\t') {
-           fprintf (stderr, "Warning: Unexpected continued value\n");
-           ADVANCE_TO_NEXT_HEADER_LINE (message->next_line);
-           continue;
-       }
+       if (message->parsing_finished)
+           break;
 
-       colon = s;
-       ADVANCE_TO (colon, ':');
+       colon = strchr (message->line, ':');
 
-       if (! WITHIN (colon) || *colon == '\n') {
-           fprintf (stderr, "Warning: Unexpected non-header line: %s\n", s);
-           ADVANCE_TO_NEXT_HEADER_LINE (message->next_line);
+       if (colon == NULL) {
+           fprintf (stderr, "Warning: Unexpected non-header line: %s\n",
+                    message->line);
+           NEXT_HEADER_LINE (NULL);
            continue;
        }
 
-       header = xstrndup (s, colon - s);
+       header = xstrndup (message->line, colon - message->line);
 
        if (message->restrict_headers &&
            ! g_hash_table_lookup_extended (message->headers,
                                            header, NULL, NULL))
        {
            free (header);
-           message->next_line = colon;
-           ADVANCE_TO_NEXT_HEADER_LINE (message->next_line);
+           NEXT_HEADER_LINE (NULL);
            continue;
        }
 
        s = colon + 1;
-       SKIP_SPACE_IN_LINE (s);
+       while (*s == ' ' || *s == '\t')
+           s++;
 
-       message->next_line = s;
-       ADVANCE_TO_NEXT_HEADER_LINE (message->next_line);
+       message->value.len = 0;
+       copy_header_unfolding (&message->value, s);
 
-       value = copy_header_value (s, message->next_line);
+       NEXT_HEADER_LINE (&message->value);
 
        match = (strcasecmp (header, header_desired) == 0);
 
-       g_hash_table_insert (message->headers, header, value);
+       g_hash_table_insert (message->headers, header,
+                            xstrdup (message->value.str));
 
        if (match)
            return value;
     }
+
+    if (message->line)
+       free (message->line);
+    message->line = NULL;
+
+    if (message->value.size) {
+       free (message->value.str);
+       message->value.str = NULL;
+       message->value.size = 0;
+       message->value.len = 0;
+    }
+
+    return NULL;
 }
index b7d27e9169b7da486b0855edccf81ec9141fee8d..449aff7143e14023ee0b97c9a27824aceeb8b52d 100644 (file)
@@ -23,6 +23,8 @@
 
 #include "notmuch.h"
 
+#define _GNU_SOURCE /* For getline */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>