refactor(store): share the codec line reader and drop the test-only settings clear

CHANGELOG.md

@@ -8,6 +8,10 @@ as the release notes.
 
 ## [Unreleased]
 
+### Changed
+
+- The session and settings files share one line reader and one value check instead of a copy each, and the settings store's test-only `clear` is gone (the tests remove the file directly). No change to what is saved or loaded. ([#73](https://github.com/ewanc26/indigo/pull/73))
+
 ### Fixed
 
 - The timeline header's button hints are drawn smaller, so "START Exit" no longer runs into the post counter on the top screen (seen in Azahar). The layout's character-width estimate is too narrow for the system font; this is the minimum change, not a font change. ([#71](https://github.com/ewanc26/indigo/pull/71))

src/store/codec_text.h

@@ -0,0 +1,36 @@
+#ifndef INDIGO_STORE_CODEC_TEXT_H
+#define INDIGO_STORE_CODEC_TEXT_H
+
+/* The pieces the key=value codecs (session, settings) share: refusing a value
+ * that would break its line, and taking a file one line at a time. Kept
+ * inline so each codec stays a single translation unit. */
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <string.h>
+
+/* True when `v` holds a line break, which a value written to a line may not. */
+static inline bool
+indigo_codec_has_break(const char *v)
+{
+    return strchr(v, '\n') != NULL || strchr(v, '\r') != NULL;
+}
+
+/* Take the line starting at *p, which ends before `end`, and move *p past its
+ * newline. The line excludes the newline. Returns false, leaving *p alone,
+ * when no newline remains: a truncated final line is not a line. */
+static inline bool
+indigo_codec_next_line(const char **p, const char *end, const char **line, size_t *len)
+{
+    const char *nl = memchr(*p, '\n', (size_t) (end - *p));
+
+    if (!nl) {
+        return false;
+    }
+    *line = *p;
+    *len = (size_t) (nl - *p);
+    *p = nl + 1;
+    return true;
+}
+
+#endif

src/store/session_codec.c

@@ -1,22 +1,18 @@
 #include "store/session_codec.h"
 
+#include "store/codec_text.h"
+
 #include <stdio.h>
 #include <string.h>
 
-static bool
-has_break(const char *v)
-{
-    return strchr(v, '\n') != NULL || strchr(v, '\r') != NULL;
-}
-
 indigo_codec_status
 indigo_session_encode(const indigo_saved_session *s, char *out, size_t cap,
                       size_t *len)
 {
     const char *vals[] = {s->service, s->session};
 
     for (size_t i = 0; i < sizeof vals / sizeof vals[0]; i++) {
-        if (has_break(vals[i])) {
+        if (indigo_codec_has_break(vals[i])) {
             return INDIGO_CODEC_CORRUPT;
         }
     }
@@ -80,40 +76,38 @@ indigo_session_decode(const char *data, size_t len, indigo_saved_session *out)
     }
 
     while (p < end) {
-        const char *nl = memchr(p, '\n', (size_t) (end - p));
+        const char *line;
         size_t line_len;
 
-        if (!nl) {
+        if (!indigo_codec_next_line(&p, end, &line, &line_len)) {
             break; /* truncated final line */
         }
-        line_len = (size_t) (nl - p);
-        if (line_len == 3 && memcmp(p, "end", 3) == 0) {
+        if (line_len == 3 && memcmp(line, "end", 3) == 0) {
             saw_end = true;
             break;
         }
 
-        const char *eq = memchr(p, '=', line_len);
+        const char *eq = memchr(line, '=', line_len);
 
         if (!eq) {
             indigo_session_wipe(out);
             return INDIGO_CODEC_CORRUPT;
         }
 
-        size_t klen = (size_t) (eq - p);
+        size_t klen = (size_t) (eq - line);
         const char *v = eq + 1;
         size_t vlen = line_len - klen - 1;
         bool ok = true;
 
-        if (klen == 7 && memcmp(p, "service", 7) == 0) {
+        if (klen == 7 && memcmp(line, "service", 7) == 0) {
             ok = assign(out->service, sizeof out->service, v, vlen);
-        } else if (klen == 7 && memcmp(p, "session", 7) == 0) {
+        } else if (klen == 7 && memcmp(line, "session", 7) == 0) {
             ok = assign(out->session, sizeof out->session, v, vlen);
         } /* unknown keys are skipped so a newer file still loads */
         if (!ok) {
             indigo_session_wipe(out);
             return INDIGO_CODEC_TOO_BIG;
         }
-        p = nl + 1;
     }
 
     if (!saw_end) {

src/store/settings_codec.c

@@ -1,14 +1,10 @@
 #include "store/settings_codec.h"
 
+#include "store/codec_text.h"
+
 #include <stdio.h>
 #include <string.h>
 
-static bool
-has_break(const char *v)
-{
-    return strchr(v, '\n') != NULL || strchr(v, '\r') != NULL;
-}
-
 void
 indigo_settings_defaults(indigo_settings *out)
 {
@@ -54,7 +50,7 @@ indigo_settings_encode(const indigo_settings *s, char *out, size_t cap,
 {
     indigo_settings c;
 
-    if (has_break(s->default_feed)) {
+    if (indigo_codec_has_break(s->default_feed)) {
         return INDIGO_CODEC_CORRUPT;
     }
     /* Encode what is on the way in, not what the caller meant: a struct filled
@@ -171,28 +167,27 @@ indigo_settings_decode(const char *data, size_t len, indigo_settings *out)
     tmp = *out;
 
     while (p < end) {
-        const char *nl = memchr(p, '\n', (size_t) (end - p));
+        const char *line;
         size_t line_len;
 
-        if (!nl) {
+        if (!indigo_codec_next_line(&p, end, &line, &line_len)) {
             break; /* truncated final line */
         }
-        line_len = (size_t) (nl - p);
-        if (line_len > 0 && p[line_len - 1] == '\r') {
+        if (line_len > 0 && line[line_len - 1] == '\r') {
             line_len--; /* tolerate a file last edited with CRLF endings */
         }
-        if (line_len == 3 && memcmp(p, "end", 3) == 0) {
+        if (line_len == 3 && memcmp(line, "end", 3) == 0) {
             saw_end = true;
             break;
         }
 
-        const char *eq = memchr(p, '=', line_len);
+        const char *eq = memchr(line, '=', line_len);
 
         if (!eq) {
             return INDIGO_CODEC_CORRUPT;
         }
 
-        size_t klen = (size_t) (eq - p);
+        size_t klen = (size_t) (eq - line);
         const char *v = eq + 1;
         size_t vlen = line_len - klen - 1;
         int num = 0;
@@ -201,12 +196,12 @@ indigo_settings_decode(const char *data, size_t len, indigo_settings *out)
         /* Every branch below ignores a value it cannot use and leaves the
          * default in place. Anything unrecognised is skipped, so a file
          * written by a newer Indigo still loads. */
-        if (klen == 5 && memcmp(p, "theme", 5) == 0) {
+        if (klen == 5 && memcmp(line, "theme", 5) == 0) {
             if (parse_number(v, vlen, (int) INDIGO_THEME_AUTO,
                              (int) INDIGO_THEME_DARK, &num)) {
                 tmp.theme = (indigo_theme) num;
             }
-        } else if (klen == 10 && memcmp(p, "text_scale", 10) == 0) {
+        } else if (klen == 10 && memcmp(line, "text_scale", 10) == 0) {
             /* A range check alone is not enough: 101 is inside 100..130 but
              * is not a scale the renderer knows how to apply. */
             if (parse_number(v, vlen, (int) INDIGO_TEXT_SCALE_SMALL,
@@ -216,33 +211,32 @@ indigo_settings_decode(const char *data, size_t len, indigo_settings *out)
                  num == (int) INDIGO_TEXT_SCALE_LARGE)) {
                 tmp.text_scale = (indigo_text_scale) num;
             }
-        } else if (klen == 13 && memcmp(p, "reduce_motion", 13) == 0) {
+        } else if (klen == 13 && memcmp(line, "reduce_motion", 13) == 0) {
             if (parse_bool(v, vlen, &flag)) {
                 tmp.reduce_motion = flag;
             }
-        } else if (klen == 8 && memcmp(p, "alt_text", 8) == 0) {
+        } else if (klen == 8 && memcmp(line, "alt_text", 8) == 0) {
             if (parse_bool(v, vlen, &flag)) {
                 tmp.alt_text = flag;
             }
-        } else if (klen == 13 && memcmp(p, "high_contrast", 13) == 0) {
+        } else if (klen == 13 && memcmp(line, "high_contrast", 13) == 0) {
             if (parse_bool(v, vlen, &flag)) {
                 tmp.high_contrast = flag;
             }
-        } else if (klen == 13 && memcmp(p, "large_targets", 13) == 0) {
+        } else if (klen == 13 && memcmp(line, "large_targets", 13) == 0) {
             if (parse_bool(v, vlen, &flag)) {
                 tmp.large_targets = flag;
             }
-        } else if (klen == 11 && memcmp(p, "diagnostics", 11) == 0) {
+        } else if (klen == 11 && memcmp(line, "diagnostics", 11) == 0) {
             if (parse_bool(v, vlen, &flag)) {
                 tmp.diagnostics = flag;
             }
-        } else if (klen == 12 && memcmp(p, "default_feed", 12) == 0) {
+        } else if (klen == 12 && memcmp(line, "default_feed", 12) == 0) {
             if (vlen < sizeof tmp.default_feed) {
                 memcpy(tmp.default_feed, v, vlen);
                 tmp.default_feed[vlen] = '\0';
             }
         }
-        p = nl + 1;
     }
 
     if (!saw_end) {

src/store/settings_store.c

@@ -35,9 +35,3 @@ indigo_settings_store_load(const char *path, indigo_settings *out)
     indigo_file_set_aside(path);
     return INDIGO_STORE_UNREADABLE;
 }
-
-indigo_store_status
-indigo_settings_store_clear(const char *path)
-{
-    return indigo_file_remove(path);
-}

src/store/settings_store.h

@@ -13,6 +13,5 @@ indigo_store_status indigo_settings_store_save(const char *path,
  * unreadable, so the caller never has to initialise it first. */
 indigo_store_status indigo_settings_store_load(const char *path,
                                                indigo_settings *out);
-indigo_store_status indigo_settings_store_clear(const char *path);
 
 #endif

tests/tests.c

@@ -1365,9 +1365,9 @@ test_settings_store(void)
     CHECK(out.high_contrast);
     CHECK(strcmp(out.default_feed, in.default_feed) == 0);
 
-    CHECK(indigo_settings_store_clear(path) == INDIGO_STORE_OK);
+    CHECK(indigo_file_remove(path) == INDIGO_STORE_OK);
     CHECK(indigo_settings_store_load(path, &out) == INDIGO_STORE_MISSING);
-    CHECK(indigo_settings_store_clear(path) == INDIGO_STORE_OK); /* idempotent */
+    CHECK(indigo_file_remove(path) == INDIGO_STORE_OK); /* idempotent */
 
     /* A damaged file is kept aside rather than destroyed, and the app still
      * gets usable defaults instead of failing to boot. */

wolfram.ref

@@ -1 +1 @@
-v0.37.0
+v0.38.1