key remap: simplify and use movable allocations

Have action.c control the key remap buflib allocation so that it can
be made movable. With memory management offloaded, core_keymap.c only
needs to deal with loading keymap files. Simplify the code there and
use buflib pinning so the file can be loaded directly into the buffer.

Change-Id: Ia654cc05ce6b286f96c1031fa4f7d4b3859a2c1a

4 files changed, 117 insertions, 151 deletions

diff --git a/apps/action.c b/apps/action.c
index 9ef10936f2..da08e29dbb 100644
--- a/apps/action.c
+++ b/apps/action.c
@@ -34,6 +34,7 @@
 #include "button.h"
 #include "action.h"
 #include "kernel.h"
+#include "core_alloc.h"
 
 #include "splash.h"
 #include "settings.h"
@@ -70,7 +71,7 @@ static action_last_t action_last =
     .wait_for_release = false,
 
 #ifndef DISABLE_ACTION_REMAP
-    .core_keymap = NULL,
+    .key_remap = 0,
 #endif
 
 #ifdef HAVE_TOUCHSCREEN
@@ -601,9 +602,8 @@ static inline void action_code_lookup(action_last_t *last, action_cur_t *cur)
 #endif
 
 #ifndef DISABLE_ACTION_REMAP
-        bool check_remap = (last->core_keymap != NULL);
         /* attempt to look up the button in user supplied remap */
-        if(check_remap && (context & CONTEXT_PLUGIN) == 0)
+        if(last->key_remap && (context & CONTEXT_PLUGIN) == 0)
         {
 #if 0 /*Disable the REMOTE context for remap for now (BUTTON_REMOTE != 0)*/
             if ((cur->button & BUTTON_REMOTE) != 0)
@@ -611,7 +611,7 @@ static inline void action_code_lookup(action_last_t *last, action_cur_t *cur)
                 context |= CONTEXT_REMOTE;
             }
 #endif
-            cur->items = last->core_keymap;
+            cur->items = core_get_data(last->key_remap);
             i = 0;
             action = ACTION_UNKNOWN;
             /* check the lut at the beginning for the desired context */
@@ -1193,66 +1193,89 @@ int get_action(int context, int timeout)
 
 int action_set_keymap(struct button_mapping* core_keymap, int count)
 {
+#ifdef DISABLE_ACTION_REMAP
+    (void)core_keymap;
+    (void)count;
+    return -1;
+#else
+    if (count <= 0 || core_keymap == NULL)
+        return action_set_keymap_handle(0, 0);
+
+    size_t keyremap_buf_size = count * sizeof(struct button_mapping);
+    int handle = core_alloc("keyremap", keyremap_buf_size);
+    if (handle < 0)
+        return -6;
 
+    memcpy(core_get_data(handle), core_keymap, keyremap_buf_size);
+    return action_set_keymap_handle(handle, count);
+#endif
+}
+
+int action_set_keymap_handle(int handle, int count)
+{
 #ifdef DISABLE_ACTION_REMAP
-    count = -1;
+    (void)core_keymap;
+    (void)count;
+    return -1;
 #else
-    if (count > 0 && core_keymap != NULL) /* saf-tey checks :) */
+    /* free an existing remap */
+    if (action_last.key_remap > 0)
+        action_last.key_remap = core_free(action_last.key_remap);
+
+    /* if clearing the remap, we're done */
+    if (count <= 0 || handle <= 0)
+        return 0;
+
+    /* validate the keymap */
+    struct button_mapping* core_keymap = core_get_data(handle);
+    struct button_mapping* entry = &core_keymap[count - 1];
+    if (entry->action_code != (int) CONTEXT_STOPSEARCHING ||
+        entry->button_code != BUTTON_NONE) /* check for sentinel at end*/
     {
-        int i = 0;
-        struct button_mapping* entry = &core_keymap[count - 1];
-        if (entry->action_code != (int) CONTEXT_STOPSEARCHING ||
-            entry->button_code != BUTTON_NONE) /* check for sentinel at end*/
-        {
-            count = -1;
-        }
+        /* missing sentinel entry */
+        return -1;
+    }
 
-        while (count > 0 && /* check the lut at the beginning for invalid offsets */
-              (entry = &core_keymap[i])->action_code != (int) CONTEXT_STOPSEARCHING)
-        {
-            
-            if ((entry->action_code & CONTEXT_REMAPPED) == CONTEXT_REMAPPED)
-            {
-                int firstbtn = entry->button_code;
-                int endpos = firstbtn + entry->pre_button_code;
-                if (firstbtn > count || firstbtn < i || endpos > count)
-                {
-                    /* offset out of bounds */
-                    count = -2;
-                    break;
-                }
+    /* check the lut at the beginning for invalid offsets */
+    for (int i = 0; i < count; ++i)
+    {
+        entry = &core_keymap[i];
+        if (entry->action_code == (int)CONTEXT_STOPSEARCHING)
+            break;
 
-                if (core_keymap[endpos].button_code != BUTTON_NONE)
-                {
-                    /* stop sentinel is not at end of action lut*/
-                    count = -3;
-                }
-            }
-            else /* something other than a context remap in the lut */
+        if ((entry->action_code & CONTEXT_REMAPPED) == CONTEXT_REMAPPED)
+        {
+            int firstbtn = entry->button_code;
+            int endpos = firstbtn + entry->pre_button_code;
+            if (firstbtn > count || firstbtn < i || endpos > count)
             {
-                count = -4;
-                break;
+                /* offset out of bounds */
+                return -2;
             }
 
-            i++;
-
-            if (i >= count) /* no sentinel in the lut */
+            if (core_keymap[endpos].button_code != BUTTON_NONE)
             {
-                count = -5;
-                break;
+                /* stop sentinel is not at end of action lut */
+                return -3;
             }
         }
+        else
+        {
+            /* something other than a context remap in the lut */
+            return -4;
+        }
 
-        if (count <= 0)
-            core_keymap = NULL;
-    }
-    else
-#endif
-    {
-        core_keymap = NULL;
+        if (i+1 >= count)
+        {
+            /* no sentinel in the lut */
+            return -5;
+        }
     }
-    action_last.core_keymap = core_keymap;
+
+    /* success */
+    action_last.key_remap = handle;
     return count;
+#endif
 }
 
 int get_custom_action(int context,int timeout,
diff --git a/apps/action.h b/apps/action.h
index 35f08a3dbd..444437edf8 100644
--- a/apps/action.h
+++ b/apps/action.h
@@ -419,7 +419,7 @@ typedef struct
     bool          wait_for_release;
 
 #ifndef DISABLE_ACTION_REMAP
-    struct button_mapping* core_keymap;
+    int         key_remap;
 #endif
 
 #ifdef HAVE_TOUCHSCREEN
@@ -449,7 +449,9 @@ bool action_userabort(int timeout);
 const struct button_mapping* get_context_mapping(int context);
 
 /* load a key map to allow buttons for actions to be remapped see: core_keymap */
-int action_set_keymap(struct button_mapping* core_button_map, int count);
+int action_set_keymap(struct button_mapping* core_keymap, int count);
+/* load keymap in a handle: takes ownership of the handle on success */
+int action_set_keymap_handle(int handle, int count);
 
 /* returns the status code variable from action.c for the button just pressed
    If button != NULL it will be set to the actual button code */
diff --git a/apps/core_keymap.c b/apps/core_keymap.c
index dbe7ae0072..9d54fcffac 100644
--- a/apps/core_keymap.c
+++ b/apps/core_keymap.c
@@ -27,119 +27,67 @@
 #include "logf.h"
 
 #if !defined(__PCTOOL__) || defined(CHECKWPS)
-static int keymap_handle = -1;
-
-static int core_alloc_keymap(size_t bufsz)
+int core_set_keyremap(struct button_mapping* core_keymap, int count)
 {
-    keymap_handle = core_alloc_ex("key remap", bufsz, &buflib_ops_locked);
-    return keymap_handle;
+    return action_set_keymap(core_keymap, count);
 }
 
-static void core_free_keymap(void)
+static int open_key_remap(const char *filename, int *countp)
 {
-    action_set_keymap(NULL, -1);
-    if (keymap_handle > 0) /* free old buffer */
+    int fd = open(filename, O_RDONLY);
+    if (fd < 0)
+        return fd;
+
+    size_t fsize = filesize(fd);
+    int count = fsize / sizeof(struct button_mapping);
+    if (count == 0 || (size_t)(count * sizeof(struct button_mapping)) != fsize)
     {
-        keymap_handle = core_free(keymap_handle);
+        logf("core_keyremap: bad filesize %d / %lu", count, (unsigned long)fsize);
+        goto error;
     }
-}
-
-/* Allocates buffer from core and copies keymap into it */
-int core_set_keyremap(struct button_mapping* core_keymap, int count)
-{
 
-    core_free_keymap();
-    if (count > 0)
+    struct button_mapping header;
+    if(read(fd, &header, sizeof(header)) != (ssize_t)sizeof(header))
     {
-        size_t bufsize = count * sizeof(struct button_mapping);
-        if (core_keymap != NULL && core_alloc_keymap(bufsize) > 0)
-        {
-            char *buf = core_get_data(keymap_handle);
-            memcpy(buf, core_keymap, bufsize);
-            count = action_set_keymap((struct button_mapping *) buf, count);
-        }
-        else
-            count = -1;
+        logf("core_keyremap: read error");
+        goto error;
     }
-    return count;
-}
-
-int core_load_key_remap(const char *filename)
-{
-    char *buf;
-    int fd = -1;
-    int count = 0;
-    size_t fsize = 0;
-    core_free_keymap();
 
-    if (filename != NULL)
-        count = open_key_remap(filename, &fd, &fsize);
-    while (count > 0)
+    if (header.action_code != KEYREMAP_VERSION ||
+        header.button_code != KEYREMAP_HEADERID ||
+        header.pre_button_code != count)
     {
-        if (core_alloc_keymap(fsize) <= 0)
-        {
-            count = -30;
-            logf("core_keymap: %d Failed to allocate buffer", count);
-            break;
-        }
-        buf = core_get_data(keymap_handle);
-        if (read(fd, buf, fsize) == (ssize_t) fsize)
-        {
-            count = action_set_keymap((struct button_mapping *) buf, count);
-        }
-        else
-        {
-            count = -40;
-            logf("core_keymap: %d Failed to read", count);
-        }
-        break;
+        logf("core_keyremap: bad header %d", count);
+        goto error;
     }
+
+    *countp = count - 1;
+    return fd;
+
+  error:
     close(fd);
-    return count;
+    return -1;
 }
 
-int open_key_remap(const char *filename, int *fd, size_t *fsize)
+int core_load_key_remap(const char *filename)
 {
-    int count = 0;
+    int count = 0; /* gcc falsely believes this may be used uninitialized */
+    int fd = open_key_remap(filename, &count);
+    if (fd < 0)
+        return -1;
 
-    while (filename && fd && fsize)
+    size_t bufsize = count * sizeof(struct button_mapping);
+    int handle = core_alloc("keyremap", bufsize);
+    if (handle > 0)
     {
-        *fsize = 0;
-        *fd = open(filename, O_RDONLY);
-        if (*fd)
-        {
-            *fsize = filesize(*fd);
-
-            count = *fsize / sizeof(struct button_mapping);
-
-            if (count * sizeof(struct button_mapping) != *fsize)
-            {
-                count = -10;
-                logf("core_keymap: %d Size mismatch", count);
-                break;
-            }
+        core_pin(handle);
+        if (read(fd, core_get_data(handle), bufsize) == (ssize_t)bufsize)
+            count = action_set_keymap_handle(handle, count);
 
-            if (count > 1)
-            {
-                struct button_mapping header = {0};
-                read(*fd, &header, sizeof(struct button_mapping));
-                if (KEYREMAP_VERSION == header.action_code &&
-                    KEYREMAP_HEADERID == header.button_code &&
-                    header.pre_button_code == count)
-                {
-                    count--;
-                    *fsize -= sizeof(struct button_mapping);
-                }
-                else /* Header mismatch */
-                {
-                    count = -20;
-                    logf("core_keymap: %d Header mismatch", count);
-                    break;
-                }
-            }
-        }
-        break;
+        core_unpin(handle);
     }
+
+    close(fd);
     return count;
 }
 
diff --git a/apps/core_keymap.h b/apps/core_keymap.h
index dad9875364..2077daa685 100644
--- a/apps/core_keymap.h
+++ b/apps/core_keymap.h
@@ -34,13 +34,6 @@
 /* Allocates core buffer, copies keymap to allow buttons for actions to be remapped*/
 int core_set_keyremap(struct button_mapping* core_keymap, int count);
 
-/* open_key_remap(filename , *fd (you must close file_descriptor), *fsize)
- * checks/strips header and returns remaining count
- * fd is opened and set to first record
- * filesize contains the size of the remaining records
-*/
-int open_key_remap(const char *filename, int *fd, size_t *filesize);
-
 /* load a remap file to allow buttons for actions to be remapped */
 int core_load_key_remap(const char *filename);