summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Sevakis <jethead71@rockbox.org>2017-03-12 20:59:44 -0400
committerMichael Sevakis <jethead71@rockbox.org>2017-03-12 21:09:16 -0400
commit70c929179b80e0657e31558e34d2bc62e1176564 (patch)
treeb359577dabe279aa2e3995f7ef57581d583e7218
parente3081b35cdeb1e568c61369e5b3b15b6e428d3c3 (diff)
downloadrockbox-70c9291.tar.gz
rockbox-70c9291.tar.bz2
rockbox-70c9291.zip
Dircache: Refine name allocation and error handling.
* 8 bits is enough to allow 260 character base names when five bytes is the minimum indirect storage size (0..255->5..260). * Don't truncate anything that's too long as that can lead to bad behavior, simply don't include the offending entry in the parent. * Set the .tinyname flag to 1 by default to indicate that the entry's name doesn't need freeing. Clear it only when allocating indirect storage. * Rename some things to help catch all instances Change-Id: Iff747b624acbb8e03ed26c24afdf0fc715fd9d99
-rw-r--r--firmware/common/dircache.c80
-rw-r--r--firmware/common/file_internal.c2
-rw-r--r--firmware/include/file_internal.h2
3 files changed, 49 insertions, 35 deletions
diff --git a/firmware/common/dircache.c b/firmware/common/dircache.c
index 08457f2da1..42585e121b 100644
--- a/firmware/common/dircache.c
+++ b/firmware/common/dircache.c
@@ -149,15 +149,18 @@ enum
DCM_LAST = DCM_PROCEED,
};
-#define MAX_TINYNAME sizeof (uint32_t)
-#define DC_MAX_NAME MIN(MAX_NAME, UINT8_MAX)
+#define MAX_TINYNAME sizeof (uint32_t)
+#define NAMELEN_ADJ (MAX_TINYNAME+1)
+#define DC_MAX_NAME (UINT8_MAX+NAMELEN_ADJ)
+#define CE_NAMESIZE(len) ((len)+NAMELEN_ADJ)
+#define NAMESIZE_CE(size) ((size)-NAMELEN_ADJ)
/* Throw some warnings if about the limits if things may not work */
-#if MAX_NAME > UINT8_MAX
+#if MAX_COMPNAME > UINT8_MAX+5
#warning Need more than 8 bits in name length bitfield
#endif
-#if DIRCACHE_LIMIT > ((1 << 24)-255)
+#if DIRCACHE_LIMIT > ((1 << 24)-1+5)
#warning Names may not be addressable with 24 bits
#endif
@@ -173,7 +176,7 @@ struct dircache_entry
union {
struct {
uint32_t name : 24; /* indirect storage (.tinyname == 0) */
- uint32_t length : 8; /* length of name indexed by 'name' */
+ uint32_t namelen : 8; /* length of name - (MAX_TINYNAME+1) */
};
unsigned char namebuf[MAX_TINYNAME]; /* direct storage (.tinyname == 1) */
};
@@ -743,7 +746,7 @@ static void entry_name_copy(char *dst, const struct dircache_entry *ce)
{
if (LIKELY(!ce->tinyname))
{
- strmemcpy(dst, get_name(ce->name), ce->length);
+ strmemcpy(dst, get_name(ce->name), CE_NAMESIZE(ce->namelen));
return;
}
@@ -876,8 +879,8 @@ static void free_name(int nameidx, size_t size)
/**
* allocate and assign a name to the entry
*/
-static bool entry_assign_name(struct dircache_entry *ce,
- const unsigned char *name, size_t size)
+static int entry_assign_name(struct dircache_entry *ce,
+ const unsigned char *name, size_t size)
{
unsigned char *copyto;
@@ -893,21 +896,21 @@ static bool entry_assign_name(struct dircache_entry *ce,
else
{
if (size > DC_MAX_NAME)
- size = DC_MAX_NAME;
+ return -ENAMETOOLONG;
int nameidx = alloc_name(size);
if (!nameidx)
- return false;
+ return -ENOSPC;
copyto = get_name(nameidx);
ce->tinyname = 0;
ce->name = nameidx;
- ce->length = size;
+ ce->namelen = NAMESIZE_CE(size);
}
memcpy(copyto, name, size);
- return true;
+ return 0;
}
/**
@@ -915,17 +918,20 @@ static bool entry_assign_name(struct dircache_entry *ce,
*/
static void entry_unassign_name(struct dircache_entry *ce)
{
- if (!ce->tinyname)
- free_name(ce->name, ce->length);
+ if (ce->tinyname)
+ return;
+
+ free_name(ce->name, CE_NAMESIZE(ce->namelen));
+ ce->tinyname = 1;
}
/**
* assign a new name to the entry
*/
-static bool entry_reassign_name(struct dircache_entry *ce,
- const unsigned char *newname)
+static int entry_reassign_name(struct dircache_entry *ce,
+ const unsigned char *newname)
{
- size_t oldlen = ce->tinyname ? 0 : ce->length;
+ size_t oldlen = ce->tinyname ? 0 : CE_NAMESIZE(ce->namelen);
size_t newlen = strlen(newname);
if (oldlen == newlen || (oldlen == 0 && newlen <= MAX_TINYNAME))
@@ -934,7 +940,7 @@ static bool entry_reassign_name(struct dircache_entry *ce,
newname, newlen);
if (newlen < MAX_TINYNAME)
*p = '\0';
- return true;
+ return 0;
}
/* needs a new name allocation; if the new name fits in the freed block,
@@ -968,7 +974,7 @@ static int alloc_entry(struct dircache_entry **res)
{
dircache.last_size = 0;
*res = NULL;
- return 0;
+ return -ENOSPC;
}
dircache.sizeused += ENTRYSIZE;
@@ -976,7 +982,7 @@ static int alloc_entry(struct dircache_entry **res)
ce->next = 0;
ce->up = 0;
ce->down = 0;
- ce->length = 0;
+ ce->tinyname = 1;
ce->frontier = FRONTIER_SETTLED;
ce->serialnum = next_serialnum();
@@ -1030,19 +1036,20 @@ static int create_entry(const char *basename,
struct dircache_entry **res)
{
int idx = alloc_entry(res);
- if (!idx)
- {
- logf("size limit reached (entry)");
- return 0; /* full */
- }
- if (!entry_assign_name(*res, basename, strlen(basename)))
+ if (idx > 0)
{
- free_orphan_entry(NULL, *res, idx);
- logf("size limit reached (name)");
- return 0; /* really full! */
+ int rc = entry_assign_name(*res, basename, strlen(basename));
+ if (rc < 0)
+ {
+ free_orphan_entry(NULL, *res, idx);
+ idx = rc;
+ }
}
+ if (idx == -ENOSPC)
+ logf("size limit reached");
+
return idx;
}
@@ -1304,8 +1311,15 @@ static void sab_process_sub(struct sab *sabp)
}
int idx = create_entry(fatentp->name, &ce);
- if (!idx)
+ if (idx <= 0)
{
+ if (idx == -ENAMETOOLONG)
+ {
+ /* not fatal; just don't include it */
+ establish_frontier(compp->idx, FRONTIER_ZONED);
+ continue;
+ }
+
sabp->quit = true;
break;
}
@@ -2328,7 +2342,7 @@ void dircache_fileop_create(struct file_base_info *dirinfop,
struct dircache_entry *ce;
int idx = create_entry(basename, &ce);
- if (idx == 0)
+ if (idx <= 0)
{
/* failed allocation; parent cache contents are not complete */
establish_frontier(dirinfop->dcfile.idx, FRONTIER_ZONED);
@@ -2427,7 +2441,7 @@ void dircache_fileop_rename(struct file_base_info *dirinfop,
insert_file_entry(dirinfop, ce);
/* lastly, update the entry name itself */
- if (entry_reassign_name(ce, basename))
+ if (entry_reassign_name(ce, basename) == 0)
{
/* it's not really the same one now so re-stamp it */
dc_serial_t serialnum = next_serialnum();
@@ -2506,7 +2520,7 @@ static ssize_t get_path_sub(int idx, struct get_path_sub_data *data)
if (len < 0)
return -2;
- cename = alloca(MAX_NAME + 1);
+ cename = alloca(DC_MAX_NAME + 1);
entry_name_copy(cename, ce);
}
else /* idx < 0 */
diff --git a/firmware/common/file_internal.c b/firmware/common/file_internal.c
index 8fee802f6f..a109563092 100644
--- a/firmware/common/file_internal.c
+++ b/firmware/common/file_internal.c
@@ -505,7 +505,7 @@ walk_path(struct pathwalk *walkp, struct pathwalk_component *compp,
if (!(compp->attr & ATTR_DIRECTORY))
return -ENOTDIR;
- if (len >= MAX_NAME)
+ if (len > MAX_COMPNAME)
return -ENAMETOOLONG;
/* check for "." and ".." */
diff --git a/firmware/include/file_internal.h b/firmware/include/file_internal.h
index bb1236aed1..5893737833 100644
--- a/firmware/include/file_internal.h
+++ b/firmware/include/file_internal.h
@@ -56,7 +56,7 @@
root + 'Music' + 'Artist' + 'Album' + 'Disc N' + filename */
#define STATIC_PATHCOMP_NUM 6
-#define MAX_NAME 255
+#define MAX_COMPNAME 260
/* unsigned value that will also hold the off_t range we need without
overflow */