From dbe20d453d5e93bd0f1188a8851c6cf4fd230b26 Mon Sep 17 00:00:00 2001 From: William Wilgus Date: Sat, 11 Nov 2023 00:01:34 -0500 Subject: 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 --- apps/playlist.c | 2 +- firmware/common/dircache.c | 89 +++++++++++++++++++++++++++++------------ firmware/common/file.c | 5 ++- firmware/common/file_internal.c | 9 +++-- firmware/common/pathfuncs.c | 69 ++++++++++++++++++++++---------- firmware/export/pathfuncs.h | 2 +- 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); -- cgit v1.2.3