On Tue, Nov 26, 2019 at 05:29:31PM +0900, AKASHI Takahiro wrote:
Unlink test for FAT file system seems to fail at test_unlink2. (When I added this test, I haven't seen any errors though.)
FYI, once the following patches are merged, file system pytests under /test/py/tests/test_fs will start to be invoked for sandbox build on Travis CI.
[1] https://lists.denx.de/pipermail/u-boot/2019-November/391743.html [2] https://lists.denx.de/pipermail/u-boot/2019-November/391742.html
Thanks, -Takahiro Akashi
for example, ===8<=== fs_obj_unlink = ['fat', '/home/akashi/tmp/uboot_sandbox_test/128MB.fat32.img']
def test_unlink2(self, u_boot_console, fs_obj_unlink): """ Test Case 2 - delete many files """ fs_type,fs_img = fs_obj_unlink with u_boot_console.log.section('Test Case 2 - unlink (many)'): output = u_boot_console.run_command('host bind 0 %s' % fs_img) for i in range(0, 20): output = u_boot_console.run_command_list([ '%srm host 0:0 dir2/0123456789abcdef%02x' % (fs_type, i), '%sls host 0:0 dir2/0123456789abcdef%02x' % (fs_type, i)]) assert('' == ''.join(output)) output = u_boot_console.run_command( '%sls host 0:0 dir2' % fs_type)assert('0 file(s), 2 dir(s)' in output)E AssertionError: assert '0 file(s), 2 dir(s)' in ' ./\r\r\n ../\r\r\n 0 0123456789abcdef11\r\r\n\r\r\n1 file(s), 2 dir(s)'
test/py/tests/test_fs/test_unlink.py:52: AssertionError ===>8===
This can happen when fat_itr_next() wrongly detects an already- deleted directory entry.
File deletion, which was added in the commit f8240ce95d64 ("fs: fat: support unlink"), is implemented by marking its entry for a short name with DELETED_FLAG, but related entry slots for a long file name are kept unmodified. (So entries will never be actually deleted from media.)
To handle this case correctly, an additional check for a directory slot will be needed in fat_itr_next().
In addition, I added extra comments about long file name and short file name format in FAT file system. Although they are not directly related to the issue, I hope it will be helpful for better understandings in general.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
fs/fat/fat.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 9e1b842dac6b..68ce65838678 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -869,6 +869,14 @@ static dir_entry *extract_vfat_name(fat_itr *itr) return NULL; }
/*
* We are now at the short file name entry.* If it is marked as deleted, just skip it.*/if (dent->name[0] == DELETED_FLAG ||
dent->name[0] == aRING)return NULL;itr->l_name[n] = '\0';
chksum = mkcksum(dent->name, dent->ext);
@@ -898,6 +906,16 @@ static int fat_itr_next(fat_itr *itr)
itr->name = NULL;
- /*
* One logical directory entry consist of following slots:* name[0] Attributes* dent[N - N]: LFN[N - 1] N|0x40 ATTR_VFAT* ...* dent[N - 2]: LFN[1] 2 ATTR_VFAT* dent[N - 1]: LFN[0] 1 ATTR_VFAT* dent[N]: SFN ATTR_ARCH*/- while (1) { dent = next_dent(itr); if (!dent)
@@ -910,7 +928,17 @@ static int fat_itr_next(fat_itr *itr) if (dent->attr & ATTR_VOLUME) { if ((dent->attr & ATTR_VFAT) == ATTR_VFAT && (dent->name[0] & LAST_LONG_ENTRY_MASK)) {
/* long file name */ dent = extract_vfat_name(itr);/** If succeeded, dent has a valid short file* name entry for the current entry.* If failed, itr points to a current bogus* entry. So after fetching a next one,* it may have a short file name entry* for this bogus entry so that we can still* check for a short name.*/ if (!dent) continue; itr->name = itr->l_name;@@ -919,8 +947,11 @@ static int fat_itr_next(fat_itr *itr) /* Volume label or VFAT entry, skip */ continue; }
}
} else if (!(dent->attr & ATTR_ARCH) &&!(dent->attr & ATTR_DIR))continue;/* short file name */break; }
-- 2.24.0