From cb3b5397b34eb6ad181b2c9d32996152f28d3974 Mon Sep 17 00:00:00 2001 From: William Wilgus Date: Thu, 23 Nov 2023 08:10:46 -0500 Subject: Revert "Extend path_append_ex to truncate compname, remove some strmemdupa" This reverts commit dbe20d453d5e93bd0f1188a8851c6cf4fd230b26. Reason for revert: Crashes ipod Classic Change-Id: I9ea329ce73383535353832d17c7c5e494e5ad516 --- firmware/common/dircache.c | 89 ++++++++++++----------------------------- firmware/common/file.c | 5 +-- firmware/common/file_internal.c | 9 ++--- firmware/common/pathfuncs.c | 69 ++++++++++---------------------- 4 files changed, 53 insertions(+), 119 deletions(-) (limited to 'firmware/common') diff --git a/firmware/common/dircache.c b/firmware/common/dircache.c index c5062d47ed..41564194d0 100644 --- a/firmware/common/dircache.c +++ b/firmware/common/dircache.c @@ -2488,81 +2488,44 @@ 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 */ - 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; + ssize_t len; + char *cename; - do /* go all the way up to the root */ + if (idx > 0) { - count++; - if (next_idx > (int)dircache.numentries) - return -1; /* ERROR! */ - next_idx = dircache_runinfo.pentry[next_idx].up; - } while (next_idx > 0); + struct dircache_entry *ce = get_entry(idx); - 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 */ - } + data->serialhash = dc_hash_serialnum(ce->serialnum, data->serialhash); - /* we have the volume name write it to the buffer */ - goto write_path_component; - /* if not MULTIVOLUME it will just be adding '/' */ + /* 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; - while (next_idx > 0 && count > 0) + cename = alloca(DC_MAX_NAME + 1); + entry_name_copy(cename, ce); + } + else /* idx < 0 */ { - 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); + len = 0; + cename = ""; - count--; - remain = count - 1; - next_idx = idx; - } - else - { - remain--; - next_idx = ce->up; - } + #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 */ + data->serialhash = dc_hash_serialnum(get_idx_dcvolp(idx)->serialnum, + data->serialhash); } - return len; + + return len + path_append(data->buf + len, PA_SEP_HARD, cename, + data->size > (size_t)len ? data->size - len : 0); } /** diff --git a/firmware/common/file.c b/firmware/common/file.c index 2012e2ff80..202410db81 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,8 +1076,7 @@ int rename(const char *old, const char *new) FILE_ERROR(ERRNO, rc * 10 - 5); } } - else if (!strncmp(newname, oldinfo.name, oldinfo.length) && /* case-only is ok */ - newname[oldinfo.length] == '\0') /* make sure of actual match */ + else if (!strcmp(newname, oldname)) /* case-only is ok */ { DEBUGF("No name change (success)\n"); rc = 0; diff --git a/firmware/common/file_internal.c b/firmware/common/file_internal.c index 8e0df576bf..a73d9beaa2 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,8 +455,7 @@ static NO_INLINE int open_path_component(struct pathwalk *walkp, if (rc > 1 && !(callflags & FF_NOISO)) iso_decode_d_name(dir_fatent.name); - if (!strncasecmp(compp->name, dir_fatent.name, compp->length) && - dir_fatent.name[compp->length] == '\0') /* make sure of actual match */ + if (!strcasecmp(compname, dir_fatent.name)) break; } @@ -475,8 +474,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", - compp->length, compp->name, rc); + DEBUGF("I/O error opening file/directory %s (%d)\n", + compname, rc); return -EIO; } diff --git a/firmware/common/pathfuncs.c b/firmware/common/pathfuncs.c index b95cbab354..fa296cc2ed 100644 --- a/firmware/common/pathfuncs.c +++ b/firmware/common/pathfuncs.c @@ -447,22 +447,10 @@ 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. @@ -470,52 +458,45 @@ static inline size_t copynchars(char *dst, size_t dst_sz, const char *src, size_ * 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 component_max, size_t bufsize) + const char *component, size_t bufsize) { size_t len; - bool check_base = (basepath_max == 0); bool separate = false; - const char *base = basepath && !check_base && basepath[0] ? basepath : buf; - + const char *base = basepath && 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 = copynchars(buf, bufsize, basepath, basepath_max); + { + len = strlcpy(buf, basepath, bufsize); + if (basepath_max < len) + { + len = basepath_max; + buf[basepath_max] = '\0'; + } + } - if (!basepath || basepath_max == 0 || check_base) + if (!basepath || !component || basepath_max == 0) separate = !len || base[len-1] != PATH_SEPCH; - else if (component[0] && component_max > 0) + else if (component[0]) separate = len && base[len-1] != PATH_SEPCH; /* caller might lie about size of buf yet use buf as the base */ @@ -528,22 +509,14 @@ 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 + copynchars(buf, bufsize, component, component_max); + return len + strlcpy(buf, component ?: "", bufsize); } -/* 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, -1u, bufsize); + return path_append_ex(buf, basepath, -1u, component, bufsize); } /* Returns the location and length of the next path component, consuming the * input in the process. -- cgit v1.2.3