On Mon, Aug 14, 2017 at 9:47 AM, Brüns, Stefan
<stefan.bru...@rwth-aachen.de> wrote:
> On Montag, 14. August 2017 15:16:15 CEST Rob Clark wrote:
>> And drop a whole lot of ugly code!
>>
>> Signed-off-by: Rob Clark <robdcl...@gmail.com>
>> ---
>> fs/fat/fat.c | 723
>> ++++++---------------------------------------------------- include/fat.h |
>> 6 -
>> 2 files changed, 75 insertions(+), 654 deletions(-)
>
> Nice, even after accounting for the ~300 lines added for the iterators :-)
>
> Two comments inline ...
>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index 69fa7f4cab..a50a10ba47 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -119,22 +119,6 @@ int fat_register_device(struct blk_desc *dev_desc, int
>> part_no) }
>>
> [...]
>> -
>> -/*
>> * Extract zero terminated short name from a directory entry.
>> */
>> static void get_name(dir_entry *dirent, char *s_name)
>> @@ -468,95 +452,6 @@ static int slot2str(dir_slot *slotptr, char *l_name,
>> int *idx) return 0;
>> }
> [...]
>> -}
>> -
>> /* Calculate short name checksum */
>> static __u8 mkcksum(const char name[8], const char ext[3])
>> {
>> @@ -572,170 +467,11 @@ static __u8 mkcksum(const char name[8], const char
>> ext[3]) return ret;
> [...]
>>
>> /*
>> * Read boot sector and volume info from a FAT filesystem
>> @@ -877,374 +613,6 @@ static int get_fs_info(fsdata *mydata)
>> return 0;
>> }
> [...]
>> -
>> - while (isdir) {
>> - int startsect = mydata->data_begin
>> - + START(dentptr) * mydata->clust_size;
>> - dir_entry dent;
>> - char *nextname = NULL;
>> -
>> - dent = *dentptr;
>> - dentptr = &dent;
>> -
>> - idx = dirdelim(subname);
>> -
>> - if (idx >= 0) {
>> - subname[idx] = '\0';
>> - nextname = subname + idx + 1;
>> - /* Handle multiple delimiters */
>> - while (ISDIRDELIM(*nextname))
>> - nextname++;
>> - if (dols && *nextname == '\0')
>> - firsttime = 0;
>> - } else {
>> - if (dols && firsttime) {
>> - firsttime = 0;
>> - } else {
>> - isdir = 0;
>> - }
>> - }
>> -
>> - if (get_dentfromdir(mydata, startsect, subname, dentptr,
>> - isdir ? 0 : dols) == NULL) {
>> - if (dols && !isdir)
>> - *size = 0;
>> - goto exit;
>> - }
>> -
>> - if (isdir && !(dentptr->attr & ATTR_DIR))
>> - goto exit;
>> -
>> - /*
>> - * If we are looking for a directory, and found a directory
>> - * type entry, and the entry is for the root directory (as
>> - * denoted by a cluster number of 0), jump back to the start
>> - * of the function, since at least on FAT12/16, the root dir
>> - * lives in a hard-coded location and needs special handling
>> - * to parse, rather than simply following the cluster linked
>> - * list in the FAT, like other directories.
>> - */
>
> Have you checked this case still works? AFAICS this is not covered in fs-
> test.sh. Examples of suitables sandbox commands are given in the commit
> message of:
Directories split across clusters is definitely something that I've
tested. Not sure if some of them had vfat names spanning a cluster,
but handling that is inherent in the design, as it never needs more
than a single dent at a time to parse vfat names.
> 18a10d46f267057ede0490ddba71c106475b4eb1 (fat: handle paths that include ../)
hmm, but it does look like I'm missing something to ../ back to root dir..
BR,
-R
>> - if (isdir && (dentptr->attr & ATTR_DIR) && !START(dentptr)) {
>> - /*
>> - * Modify the filename to remove the prefix that gets
>> - * back to the root directory, so the initial root dir
>> - * parsing code can continue from where we are without
>> - * confusion.
>> - */
>> - strcpy(fnamecopy, nextname ?: "");
>> - /*
>> - * Set up state the same way as the function does when
>> - * first started. This is required for the root dir
>> - * parsing code operates in its expected environment.
>> - */
>> - subname = "";
>> - cursect = mydata->rootdir_sect;
>> - isdir = 0;
>> - goto root_reparse;
>> - }
>> -
>> - if (idx >= 0)
>> - subname = nextname;
>> - }
>> -
>> - if (dogetsize) {
>> - *size = FAT2CPU32(dentptr->size);
>> - ret = 0;
>> - } else {
>> - ret = get_contents(mydata, dentptr, pos, buffer, maxsize,
>> size);
>> - }
>> - debug("Size: %u, got: %llu\n", FAT2CPU32(dentptr->size), *size);
>> -
>> -exit:
>> - free(mydata->fatbuf);
>> - return ret;
>> -}
>> -
>>
>> /*
>> * Directory iterator, to simplify filesystem traversal
>> @@ -1571,12 +939,6 @@ static int fat_itr_resolve(fat_itr *itr, const char
>> *path, unsigned type) return -ENOENT;
>> }
>>
>> -int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int
>> dols, - loff_t *actread)
>> -{
>> - return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0, actread);
>> -}
>> -
>> int file_fat_detectfs(void)
>> {
>> boot_sector bs;
>> @@ -1641,31 +1003,96 @@ int file_fat_detectfs(void)
>>
>> int file_fat_ls(const char *dir)
>> {
>> - loff_t size;
>> + fsdata fsdata;
>> + fat_itr itrblock, *itr = &itrblock;
>> + int files = 0, dirs = 0;
>> + int ret;
>> +
>> + ret = fat_itr_root(itr, &fsdata);
>> + if (ret)
>> + return ret;
>> +
>> + ret = fat_itr_resolve(itr, dir, TYPE_DIR);
>> + if (ret)
>> + return ret;
>> +
>> + while (fat_itr_next(itr)) {
>> + if (fat_itr_isdir(itr)) {
>> + printf(" %s/\n", itr->name);
>> + dirs++;
>> + } else {
>> + printf(" %8u %s\n",
>> + FAT2CPU32(itr->dent->size),
>> + itr->name);
>> + files++;
>> + }
>> + }
>> +
>> + printf("\n%d file(s), %d dir(s)\n\n", files, dirs);
>>
>> - return do_fat_read(dir, NULL, 0, LS_YES, &size);
>> + return 0;
>> }
>>
>> int fat_exists(const char *filename)
>> {
>> + fsdata fsdata;
>> + fat_itr itrblock, *itr = &itrblock;
>> int ret;
>> - loff_t size;
>>
>> - ret = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, &size);
>> + ret = fat_itr_root(itr, &fsdata);
>> + if (ret)
>> + return ret;
>> +
>> + ret = fat_itr_resolve(itr, filename, TYPE_ANY);
>> return ret == 0;
>> }
>>
>> int fat_size(const char *filename, loff_t *size)
>> {
>> - return do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, size);
>> + fsdata fsdata;
>> + fat_itr itrblock, *itr = &itrblock;
>> + int ret;
>> +
>> + ret = fat_itr_root(itr, &fsdata);
>> + if (ret)
>> + return ret;
>> +
>> + ret = fat_itr_resolve(itr, filename, TYPE_FILE);
>> + if (ret) {
>> + /*
>> + * Directories don't have size, but fs_size() is not
>> + * expected to fail if passed a directory path:
>> + */
>> + fat_itr_root(itr, &fsdata);
>> + if (!fat_itr_resolve(itr, filename, TYPE_DIR)) {
>> + *size = 0;
>> + return 0;
>> + }
>
> It might be simpler to resolve with (TYPE_ANY) and check the TYPE of the
> returned iterator.
>
>> + return ret;
>> + }
>> +
>> + *size = FAT2CPU32(itr->dent->size);
>> +
>> + return 0;
>> }
>>
>> int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
>> loff_t maxsize, loff_t *actread)
>> {
>> + fsdata fsdata;
>> + fat_itr itrblock, *itr = &itrblock;
>> + int ret;
>> +
>> + ret = fat_itr_root(itr, &fsdata);
>> + if (ret)
>> + return ret;
>> +
>> + ret = fat_itr_resolve(itr, filename, TYPE_FILE);
>> + if (ret)
>> + return ret;
>> +
>> printf("reading %s\n", filename);
>> - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0,
>> - actread);
>> + return get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread);
>> }
>>
>> int file_fat_read(const char *filename, void *buffer, int maxsize)
>> diff --git a/include/fat.h b/include/fat.h
>> index 6d3fc8e4a6..3e7ab9ea8d 100644
>> --- a/include/fat.h
>> +++ b/include/fat.h
>> @@ -58,12 +58,6 @@
>> */
>> #define LAST_LONG_ENTRY_MASK 0x40
>>
>> -/* Flags telling whether we should read a file or list a directory */
>> -#define LS_NO 0
>> -#define LS_YES 1
>> -#define LS_DIR 1
>> -#define LS_ROOT 2
>> -
>> #define ISDIRDELIM(c) ((c) == '/' || (c) == '\\')
>>
>> #define FSTYPE_NONE (-1)
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
