Big cleanup for appendsolution()

With this PR the appendsolution routine is extracted from the h48
solver and the new coordinate solver and made generic. This has
many advantages:
- less repetition (even if the two versions are different enough that
  *for now* it was not a big deal)
- smaller h48/solve.h file, which is already a big beast
- easier to test the appendsolution() routine separately

2 files changed, 55 insertions, 54 deletions

diff --git a/src/core/io_moves.h b/src/core/io_moves.h
index fa91e8b..8c56a9f 100644
--- a/src/core/io_moves.h
+++ b/src/core/io_moves.h
@@ -1,6 +1,26 @@
 STATIC uint8_t readmove(char);
+STATIC int64_t readmoves(const char *, size_t n, uint8_t [n]);
 STATIC uint8_t readmodifier(char);
-STATIC int64_t writemoves(size_t n, uint8_t [n], size_t m, char [m]);
+STATIC int64_t writemoves(size_t n, const uint8_t [n], size_t m, char [m]);
+
+#define FOREACH_READMOVE(ARG_BUF, ARG_MOVE, ARG_C, ARG_MAX, \
+        RET_ERROR, ARG_ACTION) \
+        const char *VAR_B; \
+        uint8_t VAR_MOVE_NOMOD, VAR_MOD; \
+        for (VAR_B = ARG_BUF, ARG_C = 0; *VAR_B != '\0'; VAR_B++, ARG_C++) { \
+                while (*VAR_B == ' ' || *VAR_B == '\t' || *VAR_B == '\n') \
+                        VAR_B++; \
+                if (*VAR_B == '\0' || ARG_C == ARG_MAX) \
+                        break; \
+                if ((VAR_MOVE_NOMOD = readmove(*VAR_B)) == UINT8_ERROR) { \
+                        LOG("Error: unknown move '%c'\n", *VAR_B); \
+                        return RET_ERROR; \
+                } \
+                if ((VAR_MOD = readmodifier(*(VAR_B+1))) != 0) \
+                        VAR_B++; \
+                ARG_MOVE = VAR_MOVE_NOMOD + VAR_MOD; \
+                ARG_ACTION \
+        }
 
 STATIC uint8_t
 readmove(char c)
@@ -39,9 +59,22 @@ readmodifier(char c)
 }
 
 STATIC int64_t
+readmoves(const char *buf, size_t n, uint8_t ret[n])
+{
+        uint8_t m;
+        uint64_t c;
+
+        FOREACH_READMOVE(buf, m, c, n, NISSY_ERROR_INVALID_MOVES,
+                ret[c] = m;
+        )
+
+        return (int64_t)c;
+}
+
+STATIC int64_t
 writemoves(
         size_t nmoves,
-        uint8_t m[nmoves],
+        const uint8_t m[nmoves],
         size_t buf_size,
         char buf[buf_size]
 )
@@ -69,7 +102,9 @@ writemoves(
                 *b = ' ';
         }
 
-        if (b != buf)
+        if (b == buf)
+                written = 1; /* Nothing written, only NULL-terminator */
+        else
                 b--; /* Remove last space */
         *b = '\0';
 
diff --git a/src/core/moves.h b/src/core/moves.h
index 820406b..e56d5c8 100644
--- a/src/core/moves.h
+++ b/src/core/moves.h
@@ -3,6 +3,7 @@
 
 STATIC_INLINE bool allowednextmove(size_t n, const uint8_t [n]);
 STATIC_INLINE uint32_t allowednextmove_mask(size_t n, const uint8_t [n]);
+STATIC bool allowedmoves(size_t n, const uint8_t [n]);
 
 STATIC_INLINE uint8_t movebase(uint8_t);
 STATIC_INLINE uint8_t moveaxis(uint8_t);
@@ -13,11 +14,9 @@ STATIC_INLINE uint32_t disable_moves(uint32_t, uint8_t);
 STATIC cube_t move(cube_t, uint8_t);
 STATIC cube_t premove(cube_t, uint8_t);
 STATIC uint8_t inverse_move(uint8_t);
-STATIC void invertmoves(size_t n, const uint8_t [n], uint8_t [n]);
-STATIC void sortparallel(size_t n, uint8_t [n]);
+STATIC void sortparallel_moves(size_t n, uint8_t [n]);
 STATIC bool are_lastmoves_singlecw(size_t n, uint8_t [n]);
 
-STATIC int readmoves(const char *, int, uint8_t *);
 STATIC cube_t applymoves(cube_t, const char *);
 
 #define FOREACH_READMOVE(ARG_BUF, ARG_MOVE, ARG_C, ARG_MAX, \
@@ -75,6 +74,18 @@ allowednextmove_mask(size_t n, const uint8_t moves[n])
         return result;
 }
 
+STATIC bool
+allowedmoves(size_t n, const uint8_t moves[n])
+{
+        uint8_t j;
+
+        for (j = 2; j < n; j++)
+                if (!allowednextmove(j, moves))
+                        return false;
+
+        return true;
+}
+
 STATIC_INLINE uint32_t 
 disable_moves(uint32_t current_result, uint8_t base_index)
 {
@@ -210,44 +221,13 @@ inverse_move(uint8_t m)
         return m - 2 * (m % 3) + 2;     
 }
 
-/*
-GCC has issues when -Wstringop-overflow is used together with O3. It produces
-warnings like the following:
-
-In function 'invertmoves',
-    inlined from 'solve_h48_appendsolution' at src/solvers/h48/solve.h:81:3,
-    inlined from 'solve_h48_dfs.isra' at src/solvers/h48/solve.h:139:3:
-warning: writing 32 bytes into a region of size 0 [-Wstringop-overflow=]
-  197 |                 ret[i] = inverse_move(moves[nmoves - i - 1]);
-      |                 ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-In function 'solve_h48_dfs.isra':
-note: at offset 192 into destination object 'invertedpremoves' of size 20
-   71 |         uint8_t invertedpremoves[MAXLEN];
-
-Clang does not give any warning.
-Someone else complained here: https://access.redhat.com/solutions/6755371
-
-To solve this issue temporarily, we use a lower optimization setting for
-this function only.
-
-TODO check if the issue is resolved
-*/
-#pragma GCC push_options
-#pragma GCC optimize ("O2")
 STATIC void
-invertmoves(size_t n, const uint8_t moves[n], uint8_t ret[n])
+sortparallel_moves(size_t n, uint8_t moves[n])
 {
         uint8_t i;
 
-        for (i = 0; i < n; i++)
-                ret[i] = inverse_move(moves[n - i - 1]);
-}
-#pragma GCC pop_options
-
-STATIC void
-sortparallel(size_t n, uint8_t moves[n])
-{
-        uint8_t i;
+        if (n < 2)
+                return;
 
         for (i = 0; i < n-1; i++)
                 if (moveaxis(moves[i]) == moveaxis(moves[i+1]) &&
@@ -268,20 +248,6 @@ are_lastmoves_singlecw(size_t n, uint8_t moves[n])
         return isbase(moves[n-1]) && (!two || isbase(moves[n-2]));
 }
 
-STATIC int
-readmoves(const char *buf, int max, uint8_t *ret)
-{
-        uint8_t m;
-        int c;
-
-        FOREACH_READMOVE(buf, m, c, max, NISSY_ERROR_INVALID_MOVES,
-                if (ret != NULL)
-                        ret[c] = m;
-        )
-
-        return c;
-}
-
 STATIC cube_t
 applymoves(cube_t cube, const char *buf)
 {