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: 18a10d46f267057ede0490ddba71c106475b4eb1 (fat: handle paths that include ../) > - 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