From 026833214a942dec42b7191fe3598e0eec0a2d9e Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 16 Dec 2020 21:30:12 -0600 Subject: [PATCH] Removing zeroing of trailing space in attribute buffers This was provided as a courtesy to hopefully make custom attributes more easy to use, however the zeroing turned out to be a bit complicated when syncing custom attributes across multiple open files. Implicitly zeroing trailing buffer space is also inconsistent with the other APIs in the filesystem, such as lfs_file_read, so this commit removes the behavior. If you need to handle differently sized custom attributes, you can either pre-zero the custom attribute buffers, or use lfs_getattr to find the on-disk size of custom attributes explicitly. --- lfs.c | 23 ++++++++-------------- lfs.h | 14 ++++++-------- tests/test_attrs.toml | 45 ++++++++++++++++++------------------------- tests/test_paths.toml | 14 +++++++------- 4 files changed, 40 insertions(+), 56 deletions(-) diff --git a/lfs.c b/lfs.c index 8376d7d..cde549a 100644 --- a/lfs.c +++ b/lfs.c @@ -638,8 +638,6 @@ static lfs_stag_t lfs_dir_getslice(lfs_t *lfs, const lfs_mdir_t *dir, return err; } - memset((uint8_t*)gbuffer + diff, 0, gsize - diff); - return tag + gdiff; } } @@ -1088,7 +1086,7 @@ static int lfs_dir_fetch(lfs_t *lfs, static int lfs_dir_getgstate(lfs_t *lfs, const lfs_mdir_t *dir, lfs_gstate_t *gstate) { - lfs_gstate_t temp; + lfs_gstate_t temp = {0}; lfs_stag_t res = lfs_dir_get(lfs, dir, LFS_MKTAG(0x7ff, 0, 0), LFS_MKTAG(LFS_TYPE_MOVESTATE, 0, sizeof(temp)), &temp); if (res < 0 && res != LFS_ERR_NOENT) { @@ -1114,10 +1112,11 @@ static int lfs_dir_getinfo(lfs_t *lfs, lfs_mdir_t *dir, } lfs_stag_t tag = lfs_dir_get(lfs, dir, LFS_MKTAG(0x780, 0x3ff, 0), - LFS_MKTAG(LFS_TYPE_NAME, id, lfs->name_max+1), info->name); + LFS_MKTAG(LFS_TYPE_NAME, id, lfs->name_max), info->name); if (tag < 0) { return (int)tag; } + info->name[lfs_tag_size(tag)] = '\0'; info->type = lfs_tag_type3(tag); @@ -2870,14 +2869,11 @@ static int lfs_file_rawsync(lfs_t *lfs, lfs_file_t *file) { for (lfs_size_t i = 0; i < f->cfg->attr_count; i++) { for (lfs_size_t j = 0; j < file->cfg->attr_count; j++) { if (f->cfg->attrs[i].type == file->cfg->attrs[i].type) { - lfs_size_t diff = lfs_min( - f->cfg->attrs[i].size, - file->cfg->attrs[i].size); memcpy(f->cfg->attrs[i].buffer, file->cfg->attrs[i].buffer, - diff); - memset((uint8_t*)f->cfg->attrs[i].buffer + diff, - 0, f->cfg->attrs[i].size - diff); + lfs_min( + f->cfg->attrs[i].size, + file->cfg->attrs[i].size)); } } } @@ -3484,11 +3480,8 @@ static int lfs_commitattr(lfs_t *lfs, const char *path, // sync attrs for (lfs_size_t i = 0; i < f->cfg->attr_count; i++) { if (f->cfg->attrs[i].type == type) { - // TODO should this zero trailing bytes? - lfs_size_t diff = lfs_min(f->cfg->attrs[i].size, size); - memcpy(f->cfg->attrs[i].buffer, buffer, diff); - memset((uint8_t*)f->cfg->attrs[i].buffer + diff, - 0, f->cfg->attrs[i].size - diff); + memcpy(f->cfg->attrs[i].buffer, buffer, + lfs_min(f->cfg->attrs[i].size, size)); } } } diff --git a/lfs.h b/lfs.h index 3b02b6a..8e3aa8b 100644 --- a/lfs.h +++ b/lfs.h @@ -300,10 +300,9 @@ struct lfs_file_config { // write occurs atomically with update to the file's contents. // // Custom attributes are uniquely identified by an 8-bit type and limited - // to LFS_ATTR_MAX bytes. When read, if the stored attribute is smaller - // than the buffer, it will be padded with zeros. If the stored attribute - // is larger, then it will be silently truncated. If the attribute is not - // found, it will be created implicitly. + // to LFS_ATTR_MAX bytes. If the stored attribute is larger than the + // provided buffer, it will be silently truncated. If no attribute is + // found, and the file is open for writing, it will be created implicitly. struct lfs_attr *attrs; // Number of custom attributes in the list @@ -471,10 +470,9 @@ int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info); // Get a custom attribute // // Custom attributes are uniquely identified by an 8-bit type and limited -// to LFS_ATTR_MAX bytes. When read, if the stored attribute is smaller than -// the buffer, it will be padded with zeros. If the stored attribute is larger, -// then it will be silently truncated. If no attribute is found, the error -// LFS_ERR_NOATTR is returned and the buffer is filled with zeros. +// to LFS_ATTR_MAX bytes. If the stored attribute is larger than the +// provided buffer, it will be silently truncated. If no attribute is found, +// the error LFS_ERR_NOATTR is returned and the buffer is filled with zeros. // // Returns the size of the attribute, or a negative error code on failure. // Note, the returned size is the size of the attribute on disk, irrespective diff --git a/tests/test_attrs.toml b/tests/test_attrs.toml index 6148463..1dc828b 100644 --- a/tests/test_attrs.toml +++ b/tests/test_attrs.toml @@ -25,7 +25,6 @@ code = ''' lfs_getattr(&lfs, "hello", 'B', buffer+4, 6) => 0; lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5; assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "\0\0\0\0\0\0", 6) == 0); assert(memcmp(buffer+10, "ccccc", 5) == 0); lfs_removeattr(&lfs, "hello", 'B') => 0; @@ -33,7 +32,6 @@ code = ''' lfs_getattr(&lfs, "hello", 'B', buffer+4, 6) => LFS_ERR_NOATTR; lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5; assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "\0\0\0\0\0\0", 6) == 0); assert(memcmp(buffer+10, "ccccc", 5) == 0); lfs_setattr(&lfs, "hello", 'B', "dddddd", 6) => 0; @@ -48,9 +46,9 @@ code = ''' lfs_getattr(&lfs, "hello", 'A', buffer, 4) => 4; lfs_getattr(&lfs, "hello", 'B', buffer+4, 6) => 3; lfs_getattr(&lfs, "hello", 'C', buffer+10, 5) => 5; - assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "eee\0\0\0", 6) == 0); - assert(memcmp(buffer+10, "ccccc", 5) == 0); + assert(memcmp(buffer, "aaaa", 4) == 0); + assert(memcmp(buffer+4, "eee", 3) == 0); + assert(memcmp(buffer+10, "ccccc", 5) == 0); lfs_setattr(&lfs, "hello", 'A', buffer, LFS_ATTR_MAX+1) => LFS_ERR_NOSPC; lfs_setattr(&lfs, "hello", 'B', "fffffffff", 9) => 0; @@ -102,17 +100,15 @@ code = ''' lfs_getattr(&lfs, "/", 'A', buffer, 4) => 4; lfs_getattr(&lfs, "/", 'B', buffer+4, 6) => 0; lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5; - assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "\0\0\0\0\0\0", 6) == 0); - assert(memcmp(buffer+10, "ccccc", 5) == 0); + assert(memcmp(buffer, "aaaa", 4) == 0); + assert(memcmp(buffer+10, "ccccc", 5) == 0); lfs_removeattr(&lfs, "/", 'B') => 0; lfs_getattr(&lfs, "/", 'A', buffer, 4) => 4; lfs_getattr(&lfs, "/", 'B', buffer+4, 6) => LFS_ERR_NOATTR; lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5; - assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "\0\0\0\0\0\0", 6) == 0); - assert(memcmp(buffer+10, "ccccc", 5) == 0); + assert(memcmp(buffer, "aaaa", 4) == 0); + assert(memcmp(buffer+10, "ccccc", 5) == 0); lfs_setattr(&lfs, "/", 'B', "dddddd", 6) => 0; lfs_getattr(&lfs, "/", 'A', buffer, 4) => 4; @@ -126,9 +122,9 @@ code = ''' lfs_getattr(&lfs, "/", 'A', buffer, 4) => 4; lfs_getattr(&lfs, "/", 'B', buffer+4, 6) => 3; lfs_getattr(&lfs, "/", 'C', buffer+10, 5) => 5; - assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "eee\0\0\0", 6) == 0); - assert(memcmp(buffer+10, "ccccc", 5) == 0); + assert(memcmp(buffer, "aaaa", 4) == 0); + assert(memcmp(buffer+4, "eee", 3) == 0); + assert(memcmp(buffer+10, "ccccc", 5) == 0); lfs_setattr(&lfs, "/", 'A', buffer, LFS_ATTR_MAX+1) => LFS_ERR_NOSPC; lfs_setattr(&lfs, "/", 'B', "fffffffff", 9) => 0; @@ -193,9 +189,8 @@ code = ''' attrs1[1].size = 6; lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0; lfs_file_close(&lfs, &file) => 0; - assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "\0\0\0\0\0\0", 6) == 0); - assert(memcmp(buffer+10, "ccccc", 5) == 0); + assert(memcmp(buffer, "aaaa", 4) == 0); + assert(memcmp(buffer+10, "ccccc", 5) == 0); attrs1[1].size = 6; lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_WRONLY, &cfg1) => 0; @@ -219,9 +214,9 @@ code = ''' attrs1[1].size = 6; lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_RDONLY, &cfg1) => 0; lfs_file_close(&lfs, &file) => 0; - assert(memcmp(buffer, "aaaa", 4) == 0); - assert(memcmp(buffer+4, "eee\0\0\0", 6) == 0); - assert(memcmp(buffer+10, "ccccc", 5) == 0); + assert(memcmp(buffer, "aaaa", 4) == 0); + assert(memcmp(buffer+4, "eee", 3) == 0); + assert(memcmp(buffer+10, "ccccc", 5) == 0); attrs1[0].size = LFS_ATTR_MAX+1; lfs_file_opencfg(&lfs, &file, "hello/hello", LFS_O_WRONLY, &cfg1) @@ -292,18 +287,16 @@ code = ''' lfs_getattr(&lfs, "hello/hello", 'B', buffer, 9) => 9; lfs_getattr(&lfs, "hello/hello", 'C', buffer+9, 9) => 5; lfs_getattr(&lfs, "hello/hello", 'D', buffer+18, 9) => LFS_ERR_NOATTR; - assert(memcmp(buffer, "fffffffff", 9) == 0); - assert(memcmp(buffer+9, "ccccc\0\0\0\0", 9) == 0); - assert(memcmp(buffer+18, "\0\0\0\0\0\0\0\0\0", 9) == 0); + assert(memcmp(buffer, "fffffffff", 9) == 0); + assert(memcmp(buffer+9, "ccccc", 5) == 0); lfs_file_write(&lfs, &file, "hi", 2) => 2; lfs_file_sync(&lfs, &file) => 0; lfs_getattr(&lfs, "hello/hello", 'B', buffer, 9) => 4; lfs_getattr(&lfs, "hello/hello", 'C', buffer+9, 9) => 0; lfs_getattr(&lfs, "hello/hello", 'D', buffer+18, 9) => 4; - assert(memcmp(buffer, "gggg\0\0\0\0\0", 9) == 0); - assert(memcmp(buffer+9, "\0\0\0\0\0\0\0\0\0", 9) == 0); - assert(memcmp(buffer+18, "hhhh\0\0\0\0\0", 9) == 0); + assert(memcmp(buffer, "gggg", 4) == 0); + assert(memcmp(buffer+18, "hhhh", 4) == 0); lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; diff --git a/tests/test_paths.toml b/tests/test_paths.toml index a7474c0..f1ab65a 100644 --- a/tests/test_paths.toml +++ b/tests/test_paths.toml @@ -95,9 +95,9 @@ code = ''' lfs_mkdir(&lfs, "coffee/../milk") => 0; lfs_stat(&lfs, "coffee/../milk", &info) => 0; - strcmp(info.name, "milk") => 0; + assert(strcmp(info.name, "milk") == 0); lfs_stat(&lfs, "milk", &info) => 0; - strcmp(info.name, "milk") => 0; + assert(strcmp(info.name, "milk") == 0); lfs_unmount(&lfs) => 0; ''' @@ -129,9 +129,9 @@ code = ''' lfs_mount(&lfs, &cfg) => 0; lfs_mkdir(&lfs, ".milk") => 0; lfs_stat(&lfs, ".milk", &info) => 0; - strcmp(info.name, ".milk") => 0; + assert(strcmp(info.name, ".milk") == 0); lfs_stat(&lfs, "tea/.././.milk", &info) => 0; - strcmp(info.name, ".milk") => 0; + assert(strcmp(info.name, ".milk") == 0); lfs_unmount(&lfs) => 0; ''' @@ -149,13 +149,13 @@ code = ''' lfs_mkdir(&lfs, "coffee/coldcoffee") => 0; lfs_stat(&lfs, "coffee/../../../../../../tea/hottea", &info) => 0; - strcmp(info.name, "hottea") => 0; + assert(strcmp(info.name, "hottea") == 0); lfs_mkdir(&lfs, "coffee/../../../../../../milk") => 0; lfs_stat(&lfs, "coffee/../../../../../../milk", &info) => 0; - strcmp(info.name, "milk") => 0; + assert(strcmp(info.name, "milk") == 0); lfs_stat(&lfs, "milk", &info) => 0; - strcmp(info.name, "milk") => 0; + assert(strcmp(info.name, "milk") == 0); lfs_unmount(&lfs) => 0; '''