Extend path_append_ex to truncate compname, remove some strmemdupa

remove some duplicated strings previously allocd off the stack

just removing string duplications that are easily handled with truncation
now available with path_append_ex()

this also has an advantage of less stack used in worst case scenarios

Change-Id: I3a43e33ef8a8c36599e4c6c036a0ccdd8ed0c883

6 files changed, 121 insertions, 55 deletions

diff --git a/apps/playlist.c b/apps/playlist.c
index 67d59d1aac..f4e8ddb104 100644
--- a/apps/playlist.c
+++ b/apps/playlist.c
@@ -589,7 +589,7 @@ static ssize_t format_track_path(char *dest, char *src, int buf_length,
         dlen = -1u;
     }
 
-    len = path_append_ex(dest, dir, dlen, src, buf_length);
+    len = path_append_ex(dest, dir, dlen, src, -1u, buf_length);
     if (len >= (size_t)buf_length)
         return -1; /* buffer too small */
 
diff --git a/firmware/common/dircache.c b/firmware/common/dircache.c
index 41564194d0..c5062d47ed 100644
--- a/firmware/common/dircache.c
+++ b/firmware/common/dircache.c
@@ -2488,44 +2488,81 @@ struct get_path_sub_data
 
 static ssize_t get_path_sub(int idx, struct get_path_sub_data *data)
 {
+
+#ifdef HAVE_MULTIVOLUME
+#define NAMEBUFLEN (MAX(VOL_MAX_LEN+1, MAX_TINYNAME+1))
+#else
+#define NAMEBUFLEN (MAX_TINYNAME+1)
+#endif
+    char namebuf[NAMEBUFLEN];
+#undef NAMEBUFLEN
+
     if (idx == 0)
         return -1; /* entry is an orphan split from any root */
 
-    ssize_t len;
-    char *cename;
+    char *cename = "";
+    size_t cename_len = (-1u);
+    ssize_t len = 0;
+    int next_idx = idx;
+    int count = 1; /* +1 for the idx incoming */
+    int remain;
 
-    if (idx > 0)
+    do /* go all the way up to the root */
     {
-        struct dircache_entry *ce = get_entry(idx);
+        count++;
+        if (next_idx > (int)dircache.numentries)
+            return -1; /* ERROR! */
+        next_idx = dircache_runinfo.pentry[next_idx].up;
+    } while (next_idx > 0);
 
-        data->serialhash = dc_hash_serialnum(ce->serialnum, data->serialhash);
+    if (next_idx < 0) /* root */
+    {
+        data->serialhash = dc_hash_serialnum(get_idx_dcvolp(next_idx)->serialnum,
+                                             data->serialhash);
+#ifdef HAVE_MULTIVOLUME
+        /* prepend the volume specifier */
+        cename = namebuf;
+        get_volume_name(IF_MV_VOL(-next_idx - 1), cename);
+#endif /* HAVE_MULTIVOLUME */
+    }
 
-        /* go all the way up then move back down from the root */
-        len = get_path_sub(ce->up, data) - 1;
-        if (len < 0)
-            return -2;
+    /* we have the volume name write it to the buffer */
+    goto write_path_component;
+    /* if not MULTIVOLUME it will just be adding '/' */
 
-        cename = alloca(DC_MAX_NAME + 1);
-        entry_name_copy(cename, ce);
-    }
-    else /* idx < 0 */
+    while (next_idx > 0 && count > 0)
     {
-        len = 0;
-        cename = "";
+        struct dircache_entry *ce = &dircache_runinfo.pentry[next_idx];
+        if (remain <= 0)
+        {
+            data->serialhash = dc_hash_serialnum(ce->serialnum, data->serialhash);
+            if (LIKELY(!ce->tinyname))
+            {
+                cename = get_name(ce->name);
+                cename_len = CE_NAMESIZE(ce->namelen);
+            }
+            else
+            {
+                cename = namebuf;
+                entry_name_copy(cename, ce);
+                cename_len = -1u;
+            }
+write_path_component:
+            len += path_append_ex(data->buf + len, PA_SEP_HARD, -1u, cename, cename_len,
+                             data->size > (size_t)len ? data->size - len : 0);
 
-    #ifdef HAVE_MULTIVOLUME
-        /* prepend the volume specifier */
-        int volume = IF_MV_VOL(-idx - 1);
-        cename = alloca(VOL_MAX_LEN+1);
-        get_volume_name(volume, cename);
-    #endif /* HAVE_MULTIVOLUME */
+            count--;
+            remain = count - 1;
+            next_idx = idx;
+        }
+        else
+        {
+            remain--;
+            next_idx = ce->up;
+        }
 
-        data->serialhash = dc_hash_serialnum(get_idx_dcvolp(idx)->serialnum,
-                                             data->serialhash);
     }
-
-    return len + path_append(data->buf + len, PA_SEP_HARD, cename,
-                             data->size > (size_t)len ? data->size - len : 0);
+    return len;
 }
 
 /**
diff --git a/firmware/common/file.c b/firmware/common/file.c
index 202410db81..2012e2ff80 100644
--- a/firmware/common/file.c
+++ b/firmware/common/file.c
@@ -1056,7 +1056,7 @@ int rename(const char *old, const char *new)
         FILE_ERROR(EINVAL, -4);
     }
 
-    const char * const oldname = strmemdupa(oldinfo.name, oldinfo.length);
+    //const char * const oldname = strmemdupa(oldinfo.name, oldinfo.length);
     const char * const newname = strmemdupa(newinfo.name, newinfo.length);
     bool is_overwrite = false;
 
@@ -1076,7 +1076,8 @@ int rename(const char *old, const char *new)
                     FILE_ERROR(ERRNO, rc * 10 - 5);
             }
         }
-        else if (!strcmp(newname, oldname)) /* case-only is ok */
+        else if (!strncmp(newname, oldinfo.name, oldinfo.length) && /* case-only is ok */
+                 newname[oldinfo.length] == '\0') /* make sure of actual match */
         {
             DEBUGF("No name change (success)\n");
             rc = 0;
diff --git a/firmware/common/file_internal.c b/firmware/common/file_internal.c
index a73d9beaa2..8e0df576bf 100644
--- a/firmware/common/file_internal.c
+++ b/firmware/common/file_internal.c
@@ -433,7 +433,7 @@ static NO_INLINE int open_path_component(struct pathwalk *walkp,
     int rc;
 
     /* create a null-terminated copy of the component name */
-    char *compname = strmemdupa(compp->name, compp->length);
+    //char *compname = strmemdupa(compp->name, compp->length);
 
     unsigned int callflags = walkp->callflags;
     struct pathwalk_component *parentp = compp->nextp;
@@ -455,7 +455,8 @@ static NO_INLINE int open_path_component(struct pathwalk *walkp,
         if (rc > 1 && !(callflags & FF_NOISO))
             iso_decode_d_name(dir_fatent.name);
 
-        if (!strcasecmp(compname, dir_fatent.name))
+        if (!strncasecmp(compp->name, dir_fatent.name, compp->length) &&
+            dir_fatent.name[compp->length] == '\0') /* make sure of actual match */
             break;
     }
 
@@ -474,8 +475,8 @@ static NO_INLINE int open_path_component(struct pathwalk *walkp,
                   &compp->info.fatfile);
     if (rc < 0)
     {
-        DEBUGF("I/O error opening file/directory %s (%d)\n",
-               compname, rc);
+        DEBUGF("I/O error opening file/directory  %.*s (%d)\n",
+               compp->length, compp->name, rc);
         return -EIO;
     }
 
diff --git a/firmware/common/pathfuncs.c b/firmware/common/pathfuncs.c
index ff3de7b616..0fbac68221 100644
--- a/firmware/common/pathfuncs.c
+++ b/firmware/common/pathfuncs.c
@@ -447,10 +447,22 @@ void path_remove_dot_segments (char *dstpath, const char *path)
     *dstp = 0;
 }
 
+/* helper function copy n chars of a string to a dst buffer of dst_sz
+ * returns the length of the string it created or attempted to create
+ */
+static inline size_t copynchars(char *dst, size_t dst_sz, const char *src, size_t src_max)
+{
+    int cpychrs = -1;
+    if (src_max < -1u) /* we could be dealing with unterminated strings */
+        cpychrs = src_max;
+    /* testing shows this to be on par with using discreet functions and safer ;) */
+    int ret = snprintf(dst, dst_sz, "%.*s", cpychrs, src);
+    if (ret >= 0)
+        return ret;
+
+    return 0; /* Error */
+}
 /* Appends one path to another, adding separators between components if needed.
- * basepath_max can be used to truncate the basepath if desired
- * NOTE: basepath is truncated after copying to the buffer so there must be enough
- * free space for the entirety of the basepath even if the resulting string would fit
  *
  * Return value and behavior is otherwise as strlcpy so that truncation may be
  * detected.
@@ -458,45 +470,52 @@ void path_remove_dot_segments (char *dstpath, const char *path)
  * For basepath and component:
  * PA_SEP_HARD adds a separator even if the base path is empty
  * PA_SEP_SOFT adds a separator only if the base path is not empty
+ *
+ * basepath_max can be used to truncate the basepath if desired
+ * component_max can be used to truncate the component if desired
+ *
+ * (Supply -1u to disable truncation)
  */
 size_t path_append_ex(char *buf, const char *basepath, size_t basepath_max,
-                   const char *component, size_t bufsize)
+                   const char *component, size_t component_max, size_t bufsize)
 {
     size_t len;
+    bool check_base = (basepath_max == 0);
     bool separate = false;
-    const char *base = basepath && basepath[0] ? basepath : buf;
+    const char *base = basepath && !check_base && basepath[0] ? basepath : buf;
+
     if (!base)
         return bufsize; /* won't work to get lengths from buf */
 
     if (!buf)
+    {
+        static char fbuf; /* fake buffer to elide later null checks */
+        buf = &fbuf;
         bufsize = 0;
-
+    }
+    if (!component)
+    {
+        check_base = true;
+        component = "";
+    }
     if (path_is_absolute(component))
     {
         /* 'component' is absolute; replace all */
         basepath = component;
+        basepath_max = component_max;
         component = "";
-        basepath_max = -1u;
     }
 
     /* if basepath is not null or empty, buffer contents are replaced,
        otherwise buf contains the base path */
-
     if (base == buf)
         len = strlen(buf);
     else
-    {
-        len = strlcpy(buf, basepath, bufsize);
-        if (basepath_max < len)
-        {
-            len = basepath_max;
-            buf[basepath_max] = '\0';
-        }
-    }
+        len = copynchars(buf, bufsize, basepath, basepath_max);
 
-    if (!basepath || !component || basepath_max == 0)
+    if (!basepath || basepath_max == 0 || check_base)
         separate = !len || base[len-1] != PATH_SEPCH;
-    else if (component[0])
+    else if (component[0] && component_max > 0)
         separate = len && base[len-1] != PATH_SEPCH;
 
     /* caller might lie about size of buf yet use buf as the base */
@@ -509,14 +528,22 @@ size_t path_append_ex(char *buf, const char *basepath, size_t basepath_max,
     if (separate && (len++, bufsize > 0) && --bufsize > 0)
         *buf++ = PATH_SEPCH;
 
-    return len + strlcpy(buf, component ?: "", bufsize);
+    return len + copynchars(buf, bufsize, component, component_max);
 }
 
-
+/* Appends one path to another, adding separators between components if needed.
+ *
+ * Return value and behavior is otherwise as strlcpy so that truncation may be
+ * detected.
+ *
+ * For basepath and component:
+ * PA_SEP_HARD adds a separator even if the base path is empty
+ * PA_SEP_SOFT adds a separator only if the base path is not empty
+ */
 size_t path_append(char *buf, const char *basepath,
                    const char *component, size_t bufsize)
 {
-    return path_append_ex(buf, basepath, -1u, component, bufsize);
+    return path_append_ex(buf, basepath, -1u, component, -1u, bufsize);
 }
 /* Returns the location and length of the next path component, consuming the
  * input in the process.
diff --git a/firmware/export/pathfuncs.h b/firmware/export/pathfuncs.h
index 1b18f22d06..022a631654 100644
--- a/firmware/export/pathfuncs.h
+++ b/firmware/export/pathfuncs.h
@@ -95,7 +95,7 @@ void path_remove_dot_segments(char *dstpath, const char *path);
 #define PA_SEP_HARD NULL /* separate even if base is empty */
 #define PA_SEP_SOFT ""   /* separate only if base is nonempty */
 size_t path_append_ex(char *buf, const char *basepath, size_t basepath_max,
-                      const char *component, size_t bufsize);
+                      const char *component, size_t component_max, size_t bufsize);
 size_t path_append(char *buffer, const char *basepath, const char *component,
                    size_t bufsize);
 ssize_t parse_path_component(const char **pathp, const char **namep);