[U-Boot] [PATCH v2 0/8] fs/fat: cleanups + readdir implementation

[U-Boot] [PATCH v2 0/8] fs/fat: cleanups + readdir implementation

[Rob Clark]

Introduce directory traversal iterators, and implement fs_readdir()
which is needed by EFI_LOADER.

The part re-working fat.c to use the directory iterators itself is
nearly a 2:1 negative diffstat, and a pretty big cleanup.  I fixed
one or two other small issues along the way.  It hasn't really been
tested with a wide variaty of different fat filesystems (if someone
has a collection of disk images to test with somewhere, let me know),
but it seems at least not worse than what it is replacing.

Sorry, I realized I forgot to re-send this series.  Small cleanup
and fixed a couple bugs, compared to v1.

v2: fix a couple issues (partition is whole disk, and traversing ".."
    up to root directory)

Rob Clark (8):
  fs/fat: split out helper to init fsdata
  fs/fat: introduce new director iterators
  fat/fs: convert to directory iterators
  fs: add fs_readdir()
  fs/fat: implement opendir/readdir/closedir
  fat/fs: remove a bunch of dead code
  fat/fs: move ls to generic implementation
  fs/fat: fix case for FAT shortnames

 disk/part.c        |   31 +-
 fs/fat/Makefile    |    4 -
 fs/fat/fat.c       | 1025 +++++++++++++++++++++-------------------------------
 fs/fat/fat_write.c |    4 +-
 fs/fat/file.c      |  183 ----------
 fs/fs.c            |  124 ++++++-
 include/fat.h      |   35 +-
 include/fs.h       |   55 +++
 include/part.h     |    4 +
 9 files changed, 622 insertions(+), 843 deletions(-)
 delete mode 100644 fs/fat/file.c

-- 
2.13.5

[U-Boot] [PATCH v2 1/8] fs/fat: split out helper to init fsdata

[Rob Clark]

Want to re-use this in fat dirent iterator in next patch.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c  | 73 +++++++++++++++++++++++++++++++++++------------------------
 include/fat.h |  1 +
 2 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 465a6875ed..e1c0a15dc7 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -808,35 +808,17 @@ exit:
 	return ret;
 }
 
-__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
-	__aligned(ARCH_DMA_MINALIGN);
-
-int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
-		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
+static int get_fs_info(fsdata *mydata)
 {
-	char fnamecopy[2048];
 	boot_sector bs;
 	volume_info volinfo;
-	fsdata datablock;
-	fsdata *mydata = &datablock;
-	dir_entry *dentptr = NULL;
-	__u16 prevcksum = 0xffff;
-	char *subname = "";
-	__u32 cursect;
-	int idx, isdir = 0;
-	int files = 0, dirs = 0;
-	int ret = -1;
-	int firsttime;
 	__u32 root_cluster = 0;
-	__u32 read_blk;
-	int rootdir_size = 0;
-	int buffer_blk_cnt;
-	int do_read;
-	__u8 *dir_ptr;
+	int ret;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
+	ret = read_bootsectandvi(&bs, &volinfo, &mydata->fatsize);
+	if (ret) {
 		debug("Error: reading boot sector\n");
-		return -1;
+		return ret;
 	}
 
 	if (mydata->fatsize == 32) {
@@ -848,8 +830,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
 
 	mydata->fat_sect = bs.reserved;
 
-	cursect = mydata->rootdir_sect
-		= mydata->fat_sect + mydata->fatlength * bs.fats;
+	mydata->rootdir_sect = mydata->fat_sect + mydata->fatlength * bs.fats;
 
 	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
 	mydata->clust_size = bs.cluster_size;
@@ -863,12 +844,12 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
 		mydata->data_begin = mydata->rootdir_sect -
 					(mydata->clust_size * 2);
 	} else {
-		rootdir_size = ((bs.dir_entries[1]  * (int)256 +
-				 bs.dir_entries[0]) *
-				 sizeof(dir_entry)) /
-				 mydata->sect_size;
+		mydata->rootdir_size = ((bs.dir_entries[1]  * (int)256 +
+					 bs.dir_entries[0]) *
+					 sizeof(dir_entry)) /
+					 mydata->sect_size;
 		mydata->data_begin = mydata->rootdir_sect +
-					rootdir_size -
+					mydata->rootdir_size -
 					(mydata->clust_size * 2);
 	}
 
@@ -893,6 +874,38 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
 	debug("Sector size: %d, cluster size: %d\n", mydata->sect_size,
 	      mydata->clust_size);
 
+	return 0;
+}
+
+__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
+	__aligned(ARCH_DMA_MINALIGN);
+
+int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
+		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
+{
+	char fnamecopy[2048];
+	fsdata datablock;
+	fsdata *mydata = &datablock;
+	dir_entry *dentptr = NULL;
+	__u16 prevcksum = 0xffff;
+	char *subname = "";
+	__u32 cursect;
+	int idx, isdir = 0;
+	int files = 0, dirs = 0;
+	int ret = -1;
+	int firsttime;
+	__u32 root_cluster = 0;
+	__u32 read_blk;
+	int rootdir_size = 0;
+	int buffer_blk_cnt;
+	int do_read;
+	__u8 *dir_ptr;
+
+	if (get_fs_info(mydata))
+		return -1;
+
+	cursect = mydata->rootdir_sect;
+
 	/* "cwd" is always the root... */
 	while (ISDIRDELIM(*filename))
 		filename++;
diff --git a/include/fat.h b/include/fat.h
index 71879f01ca..6d3fc8e4a6 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -174,6 +174,7 @@ typedef struct {
 	__u16	clust_size;	/* Size of clusters in sectors */
 	int	data_begin;	/* The sector of the first cluster, can be negative */
 	int	fatbufnum;	/* Used by get_fatent, init to -1 */
+	int	rootdir_size;
 } fsdata;
 
 typedef int	(file_detectfs_func)(void);
-- 
2.13.5

[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Rob Clark]

Untangle directory traversal into a simple iterator, to replace the
existing multi-purpose do_fat_read_at() + get_dentfromdir().

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 326 insertions(+)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index e1c0a15dc7..c72d6ca931 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -1245,6 +1245,332 @@ exit:
 	return ret;
 }
 
+
+/*
+ * Directory iterator, to simplify filesystem traversal
+ */
+
+typedef struct {
+	fsdata    *fsdata;
+	unsigned   cursect;
+	dir_entry *dent;
+	int        remaining;     /* remaining dent's in current cluster */
+	int        last_cluster;
+	int        is_root;
+
+	/* current iterator position values: */
+	char       l_name[VFAT_MAXLEN_BYTES];
+	char       s_name[14];
+	char      *name;          /* l_name if there is one, else s_name */
+
+	u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
+} fat_itr;
+
+static int fat_itr_isdir(fat_itr *itr);
+
+/**
+ * fat_itr_root() - initialize an iterator to start at the root
+ * directory
+ *
+ * @itr: iterator to initialize
+ * @fsdata: filesystem data for the partition
+ * @return 0 on success, else -errno
+ */
+static int fat_itr_root(fat_itr *itr, fsdata *fsdata)
+{
+	if (get_fs_info(fsdata))
+		return -ENXIO;
+
+	itr->fsdata = fsdata;
+	itr->cursect = fsdata->rootdir_sect;
+	itr->dent = NULL;
+	itr->remaining = 0;
+	itr->last_cluster = 0;
+	itr->is_root = 1;
+
+	return 0;
+}
+
+/**
+ * fat_itr_child() - initialize an iterator to descend into a sub-
+ * directory
+ *
+ * Initializes 'itr' to iterate the contents of the directory at
+ * the current cursor position of 'parent'.  It is an error to
+ * call this if the current cursor of 'parent' is pointing at a
+ * regular file.
+ *
+ * Note that 'itr' and 'parent' can be the same pointer if you do
+ * not need to preserve 'parent' after this call, which is useful
+ * for traversing directory structure to resolve a file/directory.
+ *
+ * @itr: iterator to initialize
+ * @parent: the iterator pointing at a directory entry in the
+ *    parent directory of the directory to iterate
+ */
+static void fat_itr_child(fat_itr *itr, fat_itr *parent)
+{
+	fsdata *mydata = parent->fsdata;  /* for silly macros */
+	unsigned clustnum = START(parent->dent);
+
+	assert(fat_itr_isdir(parent));
+
+	itr->fsdata = parent->fsdata;
+	if (clustnum > 0) {
+		itr->cursect = itr->fsdata->data_begin +
+			(clustnum * itr->fsdata->clust_size);
+	} else {
+		itr->cursect = parent->fsdata->rootdir_sect;
+	}
+	itr->dent = NULL;
+	itr->remaining = 0;
+	itr->last_cluster = 0;
+	itr->is_root = 0;
+}
+
+static void *next_cluster(fat_itr *itr)
+{
+	fsdata *mydata = itr->fsdata;  /* for silly macros */
+	int ret;
+
+	/* have we reached the end? */
+	if (itr->last_cluster)
+		return NULL;
+
+	debug("FAT read(sect=%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n",
+	      itr->cursect, itr->fsdata->clust_size, DIRENTSPERBLOCK);
+
+	/*
+	 * NOTE: do_fat_read_at() had complicated logic to deal w/
+	 * vfat names that span multiple clusters in the fat16 case,
+	 * which get_dentfromdir() probably also needed (and was
+	 * missing).  And not entirely sure what fat32 didn't have
+	 * the same issue..  We solve that by only caring about one
+	 * dent at a time and iteratively constructing the vfat long
+	 * name.
+	 */
+	ret = disk_read(itr->cursect, itr->fsdata->clust_size,
+			itr->block);
+	if (ret < 0) {
+		debug("Error: reading block\n");
+		return NULL;
+	}
+
+	if (itr->is_root && itr->fsdata->fatsize != 32) {
+		itr->cursect++;
+		if (itr->cursect - itr->fsdata->rootdir_sect >=
+		    itr->fsdata->rootdir_size) {
+			debug("cursect: 0x%x\n", itr->cursect);
+			itr->last_cluster = 1;
+		}
+	} else {
+		itr->cursect = get_fatent(itr->fsdata, itr->cursect);
+		if (CHECK_CLUST(itr->cursect, itr->fsdata->fatsize)) {
+			debug("cursect: 0x%x\n", itr->cursect);
+			itr->last_cluster = 1;
+		}
+	}
+
+	return itr->block;
+}
+
+static dir_entry *next_dent(fat_itr *itr)
+{
+	if (itr->remaining == 0) {
+		struct dir_entry *dent = next_cluster(itr);
+
+		/* have we reached the last cluster? */
+		if (!dent)
+			return NULL;
+
+		itr->remaining = itr->fsdata->sect_size / sizeof(dir_entry) - 1;
+		itr->dent = dent;
+	} else {
+		itr->remaining--;
+		itr->dent++;
+	}
+
+	/* have we reached the last valid entry? */
+	if (itr->dent->name[0] == 0)
+		return NULL;
+
+	return itr->dent;
+}
+
+static dir_entry *extract_vfat_name(fat_itr *itr)
+{
+	struct dir_entry *dent = itr->dent;
+	int seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK;
+	u8 chksum, alias_checksum = ((dir_slot *)dent)->alias_checksum;
+	int n = 0;
+
+	while (seqn--) {
+		char buf[13];
+		int idx = 0;
+
+		slot2str((dir_slot *)dent, buf, &idx);
+
+		/* shift accumulated long-name up and copy new part in: */
+		memmove(itr->l_name + idx, itr->l_name, n);
+		memcpy(itr->l_name, buf, idx);
+		n += idx;
+
+		dent = next_dent(itr);
+		if (!dent)
+			return NULL;
+	}
+
+	itr->l_name[n] = '\0';
+
+	chksum = mkcksum(dent->name, dent->ext);
+
+	/* checksum mismatch could mean deleted file, etc.. skip it: */
+	if (chksum != alias_checksum) {
+		debug("** chksum=%x, alias_checksum=%x, l_name=%s, s_name=%8s.%3s\n",
+		      chksum, alias_checksum, itr->l_name, dent->name, dent->ext);
+		return NULL;
+	}
+
+	return dent;
+}
+
+/**
+ * fat_itr_next() - step to the next entry in a directory
+ *
+ * Must be called once on a new iterator before the cursor is valid.
+ *
+ * @itr: the iterator to iterate
+ * @return boolean, 1 if success or 0 if no more entries in the
+ *    current directory
+ */
+static int fat_itr_next(fat_itr *itr)
+{
+	dir_entry *dent;
+
+	itr->name = NULL;
+
+	while (1) {
+		dent = next_dent(itr);
+		if (!dent)
+			return 0;
+
+		if (dent->name[0] == DELETED_FLAG ||
+		    dent->name[0] == aRING)
+			continue;
+
+		if (dent->attr & ATTR_VOLUME) {
+			if (vfat_enabled &&
+			    (dent->attr & ATTR_VFAT) == ATTR_VFAT &&
+			    (dent->name[0] & LAST_LONG_ENTRY_MASK)) {
+				dent = extract_vfat_name(itr);
+				if (!dent)
+					continue;
+				itr->name = itr->l_name;
+				break;
+			} else {
+				/* Volume label or VFAT entry, skip */
+				continue;
+			}
+		}
+
+		break;
+	}
+
+	get_name(dent, itr->s_name);
+	if (!itr->name)
+		itr->name = itr->s_name;
+
+	return 1;
+}
+
+/**
+ * fat_itr_isdir() - is current cursor position pointing to a directory
+ *
+ * @itr: the iterator
+ * @return true if cursor is at a directory
+ */
+static int fat_itr_isdir(fat_itr *itr)
+{
+	return !!(itr->dent->attr & ATTR_DIR);
+}
+
+/*
+ * Helpers:
+ */
+
+#define TYPE_FILE 0x1
+#define TYPE_DIR  0x2
+#define TYPE_ANY  (TYPE_FILE | TYPE_DIR)
+
+/**
+ * fat_itr_resolve() - traverse directory structure to resolve the
+ * requested path.
+ *
+ * Traverse directory structure to the requested path.  If the specified
+ * path is to a directory, this will descend into the directory and
+ * leave it iterator at the start of the directory.  If the path is to a
+ * file, it will leave the iterator in the parent directory with current
+ * cursor at file's entry in the directory.
+ *
+ * @itr: iterator initialized to root
+ * @path: the requested path
+ * @type: bitmask of allowable file types
+ * @return 0 on success or -errno
+ */
+static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)
+{
+	const char *next;
+
+	/* chomp any extra leading slashes: */
+	while (path[0] && ISDIRDELIM(path[0]))
+		path++;
+
+	/* are we at the end? */
+	if (strlen(path) == 0) {
+		if (!(type & TYPE_DIR))
+			return -ENOENT;
+		return 0;
+	}
+
+	/* find length of next path entry: */
+	next = path;
+	while (next[0] && !ISDIRDELIM(next[0]))
+		next++;
+
+	while (fat_itr_next(itr)) {
+		int match = 0;
+
+		/* check both long and short name: */
+		if (!strncasecmp(path, itr->name, next - path))
+			match = 1;
+		else if (itr->name != itr->s_name &&
+			 !strncasecmp(path, itr->s_name, (next - path)))
+			match = 1;
+
+		if (!match)
+			continue;
+
+		if (fat_itr_isdir(itr)) {
+			/* recurse into directory: */
+			fat_itr_child(itr, itr);
+			return fat_itr_resolve(itr, next, type);
+		} else if (next[0]) {
+			/*
+			 * If next is not empty then we have a case
+			 * like: /path/to/realfile/nonsense
+			 */
+			debug("bad trailing path: %s\n", next);
+			return -ENOENT;
+		} else if (!(type & TYPE_FILE)) {
+			return -ENOTDIR;
+		} else {
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
 int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int dols,
 		loff_t *actread)
 {
-- 
2.13.5

[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Rob Clark]

And drop a whole lot of ugly code!

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c  | 723 ++++++----------------------------------------------------
 include/fat.h |   6 -
 2 files changed, 75 insertions(+), 654 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index c72d6ca931..3193290434 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)
 }
 
 /*
- * Get the first occurence of a directory delimiter ('/' or '\') in a string.
- * Return index into string if found, -1 otherwise.
- */
-static int dirdelim(char *str)
-{
-	char *start = str;
-
-	while (*str != '\0') {
-		if (ISDIRDELIM(*str))
-			return str - start;
-		str++;
-	}
-	return -1;
-}
-
-/*
  * 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;
 }
 
-/*
- * Extract the full long filename starting at 'retdent' (which is really
- * a slot) into 'l_name'. If successful also copy the real directory entry
- * into 'retdent'
- * Return 0 on success, -1 otherwise.
- */
-static int
-get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
-	     dir_entry *retdent, char *l_name)
-{
-	dir_entry *realdent;
-	dir_slot *slotptr = (dir_slot *)retdent;
-	__u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ?
-							PREFETCH_BLOCKS :
-							mydata->clust_size);
-	__u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
-	int idx = 0;
-
-	if (counter > VFAT_MAXSEQ) {
-		debug("Error: VFAT name is too long\n");
-		return -1;
-	}
-
-	while ((__u8 *)slotptr < buflimit) {
-		if (counter == 0)
-			break;
-		if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) != counter)
-			return -1;
-		slotptr++;
-		counter--;
-	}
-
-	if ((__u8 *)slotptr >= buflimit) {
-		dir_slot *slotptr2;
-
-		if (curclust == 0)
-			return -1;
-		curclust = get_fatent(mydata, curclust);
-		if (CHECK_CLUST(curclust, mydata->fatsize)) {
-			debug("curclust: 0x%x\n", curclust);
-			printf("Invalid FAT entry\n");
-			return -1;
-		}
-
-		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
-				mydata->clust_size * mydata->sect_size) != 0) {
-			debug("Error: reading directory block\n");
-			return -1;
-		}
-
-		slotptr2 = (dir_slot *)get_contents_vfatname_block;
-		while (counter > 0) {
-			if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
-			    & 0xff) != counter)
-				return -1;
-			slotptr2++;
-			counter--;
-		}
-
-		/* Save the real directory entry */
-		realdent = (dir_entry *)slotptr2;
-		while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
-			slotptr2--;
-			slot2str(slotptr2, l_name, &idx);
-		}
-	} else {
-		/* Save the real directory entry */
-		realdent = (dir_entry *)slotptr;
-	}
-
-	do {
-		slotptr--;
-		if (slot2str(slotptr, l_name, &idx))
-			break;
-	} while (!(slotptr->id & LAST_LONG_ENTRY_MASK));
-
-	l_name[idx] = '\0';
-	if (*l_name == DELETED_FLAG)
-		*l_name = '\0';
-	else if (*l_name == aRING)
-		*l_name = DELETED_FLAG;
-	downcase(l_name);
-
-	/* Return the real directory entry */
-	memcpy(retdent, realdent, sizeof(dir_entry));
-
-	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;
 }
 
-/*
- * Get the directory entry associated with 'filename' from the directory
- * starting at 'startsect'
- */
+// These should probably DIAF..
 __u8 get_dentfromdir_block[MAX_CLUSTSIZE]
 	__aligned(ARCH_DMA_MINALIGN);
-
-static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
-				  char *filename, dir_entry *retdent,
-				  int dols)
-{
-	__u16 prevcksum = 0xffff;
-	__u32 curclust = START(retdent);
-	int files = 0, dirs = 0;
-
-	debug("get_dentfromdir: %s\n", filename);
-
-	while (1) {
-		dir_entry *dentptr;
-
-		int i;
-
-		if (get_cluster(mydata, curclust, get_dentfromdir_block,
-				mydata->clust_size * mydata->sect_size) != 0) {
-			debug("Error: reading directory block\n");
-			return NULL;
-		}
-
-		dentptr = (dir_entry *)get_dentfromdir_block;
-
-		for (i = 0; i < DIRENTSPERCLUST; i++) {
-			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
-
-			l_name[0] = '\0';
-			if (dentptr->name[0] == DELETED_FLAG) {
-				dentptr++;
-				continue;
-			}
-			if ((dentptr->attr & ATTR_VOLUME)) {
-				if (vfat_enabled &&
-				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
-				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
-					prevcksum = ((dir_slot *)dentptr)->alias_checksum;
-					get_vfatname(mydata, curclust,
-						     get_dentfromdir_block,
-						     dentptr, l_name);
-					if (dols) {
-						int isdir;
-						char dirc;
-						int doit = 0;
-
-						isdir = (dentptr->attr & ATTR_DIR);
-
-						if (isdir) {
-							dirs++;
-							dirc = '/';
-							doit = 1;
-						} else {
-							dirc = ' ';
-							if (l_name[0] != 0) {
-								files++;
-								doit = 1;
-							}
-						}
-						if (doit) {
-							if (dirc == ' ') {
-								printf(" %8u   %s%c\n",
-								       FAT2CPU32(dentptr->size),
-									l_name,
-									dirc);
-							} else {
-								printf("            %s%c\n",
-									l_name,
-									dirc);
-							}
-						}
-						dentptr++;
-						continue;
-					}
-					debug("vfatname: |%s|\n", l_name);
-				} else {
-					/* Volume label or VFAT entry */
-					dentptr++;
-					continue;
-				}
-			}
-			if (dentptr->name[0] == 0) {
-				if (dols) {
-					printf("\n%d file(s), %d dir(s)\n\n",
-						files, dirs);
-				}
-				debug("Dentname == NULL - %d\n", i);
-				return NULL;
-			}
-			if (vfat_enabled) {
-				__u8 csum = mkcksum(dentptr->name, dentptr->ext);
-				if (dols && csum == prevcksum) {
-					prevcksum = 0xffff;
-					dentptr++;
-					continue;
-				}
-			}
-
-			get_name(dentptr, s_name);
-			if (dols) {
-				int isdir = (dentptr->attr & ATTR_DIR);
-				char dirc;
-				int doit = 0;
-
-				if (isdir) {
-					dirs++;
-					dirc = '/';
-					doit = 1;
-				} else {
-					dirc = ' ';
-					if (s_name[0] != 0) {
-						files++;
-						doit = 1;
-					}
-				}
-
-				if (doit) {
-					if (dirc == ' ') {
-						printf(" %8u   %s%c\n",
-						       FAT2CPU32(dentptr->size),
-							s_name, dirc);
-					} else {
-						printf("            %s%c\n",
-							s_name, dirc);
-					}
-				}
-
-				dentptr++;
-				continue;
-			}
-
-			if (strcmp(filename, s_name)
-			    && strcmp(filename, l_name)) {
-				debug("Mismatch: |%s|%s|\n", s_name, l_name);
-				dentptr++;
-				continue;
-			}
-
-			memcpy(retdent, dentptr, sizeof(dir_entry));
-
-			debug("DentName: %s", s_name);
-			debug(", start: 0x%x", START(dentptr));
-			debug(", size:  0x%x %s\n",
-			      FAT2CPU32(dentptr->size),
-			      (dentptr->attr & ATTR_DIR) ? "(DIR)" : "");
-
-			return retdent;
-		}
-
-		curclust = get_fatent(mydata, curclust);
-		if (CHECK_CLUST(curclust, mydata->fatsize)) {
-			debug("curclust: 0x%x\n", curclust);
-			printf("Invalid FAT entry\n");
-			return NULL;
-		}
-	}
-
-	return NULL;
-}
+__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
+	__aligned(ARCH_DMA_MINALIGN);
 
 /*
  * Read boot sector and volume info from a FAT filesystem
@@ -877,374 +613,6 @@ static int get_fs_info(fsdata *mydata)
 	return 0;
 }
 
-__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
-	__aligned(ARCH_DMA_MINALIGN);
-
-int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
-		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
-{
-	char fnamecopy[2048];
-	fsdata datablock;
-	fsdata *mydata = &datablock;
-	dir_entry *dentptr = NULL;
-	__u16 prevcksum = 0xffff;
-	char *subname = "";
-	__u32 cursect;
-	int idx, isdir = 0;
-	int files = 0, dirs = 0;
-	int ret = -1;
-	int firsttime;
-	__u32 root_cluster = 0;
-	__u32 read_blk;
-	int rootdir_size = 0;
-	int buffer_blk_cnt;
-	int do_read;
-	__u8 *dir_ptr;
-
-	if (get_fs_info(mydata))
-		return -1;
-
-	cursect = mydata->rootdir_sect;
-
-	/* "cwd" is always the root... */
-	while (ISDIRDELIM(*filename))
-		filename++;
-
-	/* Make a copy of the filename and convert it to lowercase */
-	strcpy(fnamecopy, filename);
-	downcase(fnamecopy);
-
-root_reparse:
-	if (*fnamecopy == '\0') {
-		if (!dols)
-			goto exit;
-
-		dols = LS_ROOT;
-	} else if ((idx = dirdelim(fnamecopy)) >= 0) {
-		isdir = 1;
-		fnamecopy[idx] = '\0';
-		subname = fnamecopy + idx + 1;
-
-		/* Handle multiple delimiters */
-		while (ISDIRDELIM(*subname))
-			subname++;
-	} else if (dols) {
-		isdir = 1;
-	}
-
-	buffer_blk_cnt = 0;
-	firsttime = 1;
-	while (1) {
-		int i;
-
-		if (mydata->fatsize == 32 || firsttime) {
-			dir_ptr = do_fat_read_at_block;
-			firsttime = 0;
-		} else {
-			/**
-			 * FAT16 sector buffer modification:
-			 * Each loop, the second buffered block is moved to
-			 * the buffer begin, and two next sectors are read
-			 * next to the previously moved one. So the sector
-			 * buffer keeps always 3 sectors for fat16.
-			 * And the current sector is the buffer second sector
-			 * beside the "firsttime" read, when it is the first one.
-			 *
-			 * PREFETCH_BLOCKS is 2 for FAT16 == loop[0:1]
-			 * n = computed root dir sector
-			 * loop |  cursect-1  | cursect    | cursect+1  |
-			 *   0  |  sector n+0 | sector n+1 | none       |
-			 *   1  |  none       | sector n+0 | sector n+1 |
-			 *   0  |  sector n+1 | sector n+2 | sector n+3 |
-			 *   1  |  sector n+3 | ...
-			*/
-			dir_ptr = (do_fat_read_at_block + mydata->sect_size);
-			memcpy(do_fat_read_at_block, dir_ptr, mydata->sect_size);
-		}
-
-		do_read = 1;
-
-		if (mydata->fatsize == 32 && buffer_blk_cnt)
-			do_read = 0;
-
-		if (do_read) {
-			read_blk = (mydata->fatsize == 32) ?
-				    mydata->clust_size : PREFETCH_BLOCKS;
-
-			debug("FAT read(sect=%d, cnt:%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n",
-				cursect, read_blk, mydata->clust_size, DIRENTSPERBLOCK);
-
-			if (disk_read(cursect, read_blk, dir_ptr) < 0) {
-				debug("Error: reading rootdir block\n");
-				goto exit;
-			}
-
-			dentptr = (dir_entry *)dir_ptr;
-		}
-
-		for (i = 0; i < DIRENTSPERBLOCK; i++) {
-			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
-			__u8 csum;
-
-			l_name[0] = '\0';
-			if (dentptr->name[0] == DELETED_FLAG) {
-				dentptr++;
-				continue;
-			}
-
-			if (vfat_enabled)
-				csum = mkcksum(dentptr->name, dentptr->ext);
-
-			if (dentptr->attr & ATTR_VOLUME) {
-				if (vfat_enabled &&
-				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
-				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
-					prevcksum =
-						((dir_slot *)dentptr)->alias_checksum;
-
-					get_vfatname(mydata,
-						     root_cluster,
-						     dir_ptr,
-						     dentptr, l_name);
-
-					if (dols == LS_ROOT) {
-						char dirc;
-						int doit = 0;
-						int isdir =
-							(dentptr->attr & ATTR_DIR);
-
-						if (isdir) {
-							dirs++;
-							dirc = '/';
-							doit = 1;
-						} else {
-							dirc = ' ';
-							if (l_name[0] != 0) {
-								files++;
-								doit = 1;
-							}
-						}
-						if (doit) {
-							if (dirc == ' ') {
-								printf(" %8u   %s%c\n",
-								       FAT2CPU32(dentptr->size),
-									l_name,
-									dirc);
-							} else {
-								printf("            %s%c\n",
-									l_name,
-									dirc);
-							}
-						}
-						dentptr++;
-						continue;
-					}
-					debug("Rootvfatname: |%s|\n",
-					       l_name);
-				} else {
-					/* Volume label or VFAT entry */
-					dentptr++;
-					continue;
-				}
-			} else if (dentptr->name[0] == 0) {
-				debug("RootDentname == NULL - %d\n", i);
-				if (dols == LS_ROOT) {
-					printf("\n%d file(s), %d dir(s)\n\n",
-						files, dirs);
-					ret = 0;
-				}
-				goto exit;
-			}
-			else if (vfat_enabled &&
-				 dols == LS_ROOT && csum == prevcksum) {
-				prevcksum = 0xffff;
-				dentptr++;
-				continue;
-			}
-
-			get_name(dentptr, s_name);
-
-			if (dols == LS_ROOT) {
-				int isdir = (dentptr->attr & ATTR_DIR);
-				char dirc;
-				int doit = 0;
-
-				if (isdir) {
-					dirc = '/';
-					if (s_name[0] != 0) {
-						dirs++;
-						doit = 1;
-					}
-				} else {
-					dirc = ' ';
-					if (s_name[0] != 0) {
-						files++;
-						doit = 1;
-					}
-				}
-				if (doit) {
-					if (dirc == ' ') {
-						printf(" %8u   %s%c\n",
-						       FAT2CPU32(dentptr->size),
-							s_name, dirc);
-					} else {
-						printf("            %s%c\n",
-							s_name, dirc);
-					}
-				}
-				dentptr++;
-				continue;
-			}
-
-			if (strcmp(fnamecopy, s_name)
-			    && strcmp(fnamecopy, l_name)) {
-				debug("RootMismatch: |%s|%s|\n", s_name,
-				       l_name);
-				dentptr++;
-				continue;
-			}
-
-			if (isdir && !(dentptr->attr & ATTR_DIR))
-				goto exit;
-
-			debug("RootName: %s", s_name);
-			debug(", start: 0x%x", START(dentptr));
-			debug(", size:  0x%x %s\n",
-			       FAT2CPU32(dentptr->size),
-			       isdir ? "(DIR)" : "");
-
-			goto rootdir_done;	/* We got a match */
-		}
-		debug("END LOOP: buffer_blk_cnt=%d   clust_size=%d\n", buffer_blk_cnt,
-		       mydata->clust_size);
-
-		/*
-		 * On FAT32 we must fetch the FAT entries for the next
-		 * root directory clusters when a cluster has been
-		 * completely processed.
-		 */
-		++buffer_blk_cnt;
-		int rootdir_end = 0;
-		if (mydata->fatsize == 32) {
-			if (buffer_blk_cnt == mydata->clust_size) {
-				int nxtsect = 0;
-				int nxt_clust = 0;
-
-				nxt_clust = get_fatent(mydata, root_cluster);
-				rootdir_end = CHECK_CLUST(nxt_clust, 32);
-
-				nxtsect = mydata->data_begin +
-					(nxt_clust * mydata->clust_size);
-
-				root_cluster = nxt_clust;
-
-				cursect = nxtsect;
-				buffer_blk_cnt = 0;
-			}
-		} else {
-			if (buffer_blk_cnt == PREFETCH_BLOCKS)
-				buffer_blk_cnt = 0;
-
-			rootdir_end = (++cursect - mydata->rootdir_sect >=
-				       rootdir_size);
-		}
-
-		/* If end of rootdir reached */
-		if (rootdir_end) {
-			if (dols == LS_ROOT) {
-				printf("\n%d file(s), %d dir(s)\n\n",
-				       files, dirs);
-				*size = 0;
-			}
-			goto exit;
-		}
-	}
-rootdir_done:
-
-	firsttime = 1;
-
-	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.
-		 */
-		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;
+		}
+		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)
-- 
2.13.5

[U-Boot] [PATCH v2 4/8] fs: add fs_readdir()

[Rob Clark]

Needed to support efi file protocol.  The fallback.efi loader wants
to be able to read the contents of the /EFI directory to find an OS
to boot.

Modelled after POSIX opendir()/readdir()/closedir().  Unlike the other
fs APIs, this is stateful (ie. state is held in the FS_DIR "directory
stream"), to avoid re-traversing of the directory structure at each
step.  The directory stream must be released with closedir() when it
is no longer needed.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 disk/part.c    | 31 ++++++++++++--------
 fs/fs.c        | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/fs.h   | 55 +++++++++++++++++++++++++++++++++++
 include/part.h |  4 +++
 4 files changed, 169 insertions(+), 12 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index c67fdacc79..aa9183d696 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int part,
 	return -1;
 }
 
+int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info)
+{
+	info->start = 0;
+	info->size = dev_desc->lba;
+	info->blksz = dev_desc->blksz;
+	info->bootable = 0;
+	strcpy((char *)info->type, BOOT_PART_TYPE);
+	strcpy((char *)info->name, "Whole Disk");
+#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
+	info->uuid[0] = 0;
+#endif
+#ifdef CONFIG_PARTITION_TYPE_GUID
+	info->type_guid[0] = 0;
+#endif
+
+	return 0;
+}
+
 int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str,
 			  struct blk_desc **dev_desc)
 {
@@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 
 		(*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);
 
-		info->start = 0;
-		info->size = (*dev_desc)->lba;
-		info->blksz = (*dev_desc)->blksz;
-		info->bootable = 0;
-		strcpy((char *)info->type, BOOT_PART_TYPE);
-		strcpy((char *)info->name, "Whole Disk");
-#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
-		info->uuid[0] = 0;
-#endif
-#ifdef CONFIG_PARTITION_TYPE_GUID
-		info->type_guid[0] = 0;
-#endif
+		part_get_info_whole_disk(*dev_desc, info);
 
 		ret = 0;
 		goto cleanup;
diff --git a/fs/fs.c b/fs/fs.c
index 13cd3626c6..441c880654 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -21,6 +21,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 static struct blk_desc *fs_dev_desc;
+static int fs_dev_part;
 static disk_partition_t fs_partition;
 static int fs_type = FS_TYPE_ANY;
 
@@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)
 	return -1;
 }
 
+static inline int fs_opendir_unsupported(const char *filename, FS_DIR **dirp)
+{
+	return -EACCES;
+}
+
 struct fstype_info {
 	int fstype;
 	char *name;
@@ -92,6 +98,9 @@ struct fstype_info {
 		     loff_t len, loff_t *actwrite);
 	void (*close)(void);
 	int (*uuid)(char *uuid_str);
+	int (*opendir)(const char *filename, FS_DIR **dirp);
+	int (*readdir)(FS_DIR *dirp);
+	void (*closedir)(FS_DIR *dirp);
 };
 
 static struct fstype_info fstypes[] = {
@@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_unsupported,
 #endif
 		.uuid = fs_uuid_unsupported,
+		.opendir = fs_opendir_unsupported,
 	},
 #endif
 #ifdef CONFIG_FS_EXT4
@@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_unsupported,
 #endif
 		.uuid = ext4fs_uuid,
+		.opendir = fs_opendir_unsupported,
 	},
 #endif
 #ifdef CONFIG_SANDBOX
@@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {
 		.read = fs_read_sandbox,
 		.write = fs_write_sandbox,
 		.uuid = fs_uuid_unsupported,
+		.opendir = fs_opendir_unsupported,
 	},
 #endif
 #ifdef CONFIG_CMD_UBIFS
@@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {
 		.read = ubifs_read,
 		.write = fs_write_unsupported,
 		.uuid = fs_uuid_unsupported,
+		.opendir = fs_opendir_unsupported,
 	},
 #endif
 	{
@@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {
 		.read = fs_read_unsupported,
 		.write = fs_write_unsupported,
 		.uuid = fs_uuid_unsupported,
+		.opendir = fs_opendir_unsupported,
 	},
 };
 
@@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
 
 		if (!info->probe(fs_dev_desc, &fs_partition)) {
 			fs_type = info->fstype;
+			fs_dev_part = part;
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
+/* set current blk device w/ blk_desc + partition # */
+int fs_set_blk_dev2(struct blk_desc *desc, int part)
+{
+	struct fstype_info *info;
+	int ret, i;
+
+	if (part >= 1)
+		ret = part_get_info(desc, part, &fs_partition);
+	else
+		ret = part_get_info_whole_disk(desc, &fs_partition);
+	if (ret)
+		return ret;
+	fs_dev_desc = desc;
+
+	for (i = 0, info = fstypes; i < ARRAY_SIZE(fstypes); i++, info++) {
+		if (!info->probe(fs_dev_desc, &fs_partition)) {
+			fs_type = info->fstype;
 			return 0;
 		}
 	}
@@ -334,6 +373,58 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
 	return ret;
 }
 
+FS_DIR *fs_opendir(const char *filename)
+{
+	struct fstype_info *info = fs_get_info(fs_type);
+	FS_DIR *dirp = NULL;
+	int ret;
+
+	ret = info->opendir(filename, &dirp);
+	fs_close();
+	if (ret) {
+		errno = -ret;
+		return NULL;
+	}
+
+	dirp->desc = fs_dev_desc;
+	dirp->part = fs_dev_part;
+
+	return dirp;
+}
+
+struct fs_dirent *fs_readdir(FS_DIR *dirp)
+{
+	struct fstype_info *info;
+	int ret;
+
+	fs_set_blk_dev2(dirp->desc, dirp->part);
+	info = fs_get_info(fs_type);
+
+	memset(&dirp->dirent, 0, sizeof(dirp->dirent));
+
+	ret = info->readdir(dirp);
+	fs_close();
+	if (ret)
+		return NULL;
+
+	return &dirp->dirent;;
+}
+
+void fs_closedir(FS_DIR *dirp)
+{
+	struct fstype_info *info;
+
+	if (!dirp)
+		return;
+
+	fs_set_blk_dev2(dirp->desc, dirp->part);
+	info = fs_get_info(fs_type);
+
+	info->closedir(dirp);
+	fs_close();
+}
+
+
 int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype)
 {
diff --git a/include/fs.h b/include/fs.h
index 2f2aca8378..0a6a366078 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -26,6 +26,8 @@
  */
 int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);
 
+int fs_set_blk_dev2(struct blk_desc *desc, int part);
+
 /*
  * Print the list of files on the partition previously set by fs_set_blk_dev(),
  * in directory "dirname".
@@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
 int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
 	     loff_t *actwrite);
 
+/* Add additional FS_DT_* as supported by additional filesystems:*/
+#define FS_DT_DIR  0x4       /* directory */
+#define FS_DT_REG  0x8       /* regular file */
+
+/*
+ * A directory entry.
+ */
+struct fs_dirent {
+	unsigned type;       /* one of FS_DT_* */
+	loff_t size;
+	char name[256];
+};
+
+typedef struct _FS_DIR FS_DIR;
+
+/*
+ * fs_opendir - Open a directory
+ *
+ * @filename: the path to directory to open
+ * @return a pointer to the directory stream or NULL on error and errno
+ *    set appropriately
+ */
+FS_DIR *fs_opendir(const char *filename);
+
+/*
+ * fs_readdir - Read the next directory entry in the directory stream.
+ *
+ * @dirp: the directory stream
+ * @return the next directory entry (only valid until next fs_readdir() or
+ *    fs_closedir() call, do not attempt to free()) or NULL if the end of
+ *    the directory is reached.
+ */
+struct fs_dirent *fs_readdir(FS_DIR *dirp);
+
+/*
+ * fs_closedir - close a directory stream
+ *
+ * @dirp: the directory stream
+ */
+void fs_closedir(FS_DIR *dirp);
+
+/*
+ * private to fs implementations, would be in fs.c but we need to let
+ * implementations subclass:
+ */
+
+struct _FS_DIR {
+	struct fs_dirent dirent;
+	/* private to fs layer: */
+	struct blk_desc *desc;
+	int part;
+};
+
 /*
  * Common implementation for various filesystem commands, optionally limited
  * to a specific filesystem type via the fstype parameter.
diff --git a/include/part.h b/include/part.h
index 0cd803a933..48e8ff6d8a 100644
--- a/include/part.h
+++ b/include/part.h
@@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc **blk_devp);
 
 /* disk/part.c */
 int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info);
+int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info);
 void part_print(struct blk_desc *dev_desc);
 void part_init(struct blk_desc *dev_desc);
 void dev_print(struct blk_desc *dev_desc);
@@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int dev) { return NULL; }
 
 static inline int part_get_info(struct blk_desc *dev_desc, int part,
 				disk_partition_t *info) { return -1; }
+static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,
+					   disk_partition_t *info)
+{ return -1; }
 static inline void part_print(struct blk_desc *dev_desc) {}
 static inline void part_init(struct blk_desc *dev_desc) {}
 static inline void dev_print(struct blk_desc *dev_desc) {}
-- 
2.13.5

[U-Boot] [PATCH v2 5/8] fs/fat: implement opendir/readdir/closedir

[Rob Clark]

Implement the readdir interface using the directory iterators.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 3193290434..d30ef3903b 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -14,6 +14,7 @@
 #include <config.h>
 #include <exports.h>
 #include <fat.h>
+#include <fs.h>
 #include <asm/byteorder.h>
 #include <part.h>
 #include <malloc.h>
@@ -1119,6 +1120,61 @@ int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
 	return ret;
 }
 
+typedef struct {
+	FS_DIR parent;
+	fsdata fsdata;
+	fat_itr itr;
+} fat_dir;
+
+int fat_opendir(const char *filename, FS_DIR **dirp)
+{
+	fat_dir *dir = malloc(sizeof(*dir));
+	int ret;
+
+	if (!dir)
+		return -ENOMEM;
+
+	ret = fat_itr_root(&dir->itr, &dir->fsdata);
+	if (ret)
+		goto fail;
+
+	ret = fat_itr_resolve(&dir->itr, filename, TYPE_DIR);
+	if (ret)
+		goto fail;
+
+	*dirp = (FS_DIR *)dir;
+	return 0;
+
+fail:
+	free(dir);
+	return ret;
+}
+
+int fat_readdir(FS_DIR *dirp)
+{
+	fat_dir *dir = (fat_dir *)dirp;
+	struct fs_dirent *dent = &dirp->dirent;
+
+	if (!fat_itr_next(&dir->itr))
+		return -ENOENT;
+
+	strcpy(dent->name, dir->itr.name);
+	if (fat_itr_isdir(&dir->itr)) {
+		dent->type = FS_DT_DIR;
+	} else {
+		dent->type = FS_DT_REG;
+		dent->size = FAT2CPU32(dir->itr.dent->size);
+	}
+
+	return 0;
+}
+
+void fat_closedir(FS_DIR *dirp)
+{
+	fat_dir *dir = (fat_dir *)dirp;
+	free(dir);
+}
+
 void fat_close(void)
 {
 }
-- 
2.13.5

[U-Boot] [PATCH v2 6/8] fat/fs: remove a bunch of dead code

[Rob Clark]

Spotted by chance, when trying to remove file_fat_ls(), I noticed there
were some dead users of the API.

Signed-off-by: Rob Clark <robdclark@gmail.com>
Acked-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
---
 fs/fat/Makefile |   4 --
 fs/fat/file.c   | 183 --------------------------------------------------------
 include/fat.h   |  20 -------
 3 files changed, 207 deletions(-)
 delete mode 100644 fs/fat/file.c

diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index b60e8486c4..3e2a6b01a8 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -5,7 +5,3 @@
 
 obj-$(CONFIG_FS_FAT)	:= fat.o
 obj-$(CONFIG_FAT_WRITE):= fat_write.o
-
-ifndef CONFIG_SPL_BUILD
-obj-$(CONFIG_FS_FAT)	+= file.o
-endif
diff --git a/fs/fat/file.c b/fs/fat/file.c
deleted file mode 100644
index 89706117b9..0000000000
--- a/fs/fat/file.c
+++ /dev/null
@@ -1,183 +0,0 @@
-/*
- * file.c
- *
- * Mini "VFS" by Marcus Sundberg
- *
- * 2002-07-28 - rjones at nexus-tech.net - ported to ppcboot v1.1.6
- * 2003-03-10 - kharris at nexus-tech.net - ported to uboot
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#include <common.h>
-#include <config.h>
-#include <malloc.h>
-#include <fat.h>
-#include <linux/stat.h>
-#include <linux/time.h>
-
-/* Supported filesystems */
-static const struct filesystem filesystems[] = {
-	{ file_fat_detectfs,  file_fat_ls,  file_fat_read,  "FAT" },
-};
-#define NUM_FILESYS	(sizeof(filesystems)/sizeof(struct filesystem))
-
-/* The filesystem which was last detected */
-static int current_filesystem = FSTYPE_NONE;
-
-/* The current working directory */
-#define CWD_LEN		511
-char file_cwd[CWD_LEN+1] = "/";
-
-const char *
-file_getfsname(int idx)
-{
-	if (idx < 0 || idx >= NUM_FILESYS)
-		return NULL;
-
-	return filesystems[idx].name;
-}
-
-static void
-pathcpy(char *dest, const char *src)
-{
-	char *origdest = dest;
-
-	do {
-		if (dest-file_cwd >= CWD_LEN) {
-			*dest = '\0';
-			return;
-		}
-		*(dest) = *(src);
-		if (*src == '\0') {
-			if (dest-- != origdest && ISDIRDELIM(*dest)) {
-				*dest = '\0';
-			}
-			return;
-		}
-		++dest;
-
-		if (ISDIRDELIM(*src))
-			while (ISDIRDELIM(*src)) src++;
-		else
-			src++;
-	} while (1);
-}
-
-int
-file_cd(const char *path)
-{
-	if (ISDIRDELIM(*path)) {
-		while (ISDIRDELIM(*path)) path++;
-		strncpy(file_cwd+1, path, CWD_LEN-1);
-	} else {
-		const char *origpath = path;
-		char *tmpstr = file_cwd;
-		int back = 0;
-
-		while (*tmpstr != '\0') tmpstr++;
-		do {
-			tmpstr--;
-		} while (ISDIRDELIM(*tmpstr));
-
-		while (*path == '.') {
-			path++;
-			while (*path == '.') {
-				path++;
-				back++;
-			}
-			if (*path != '\0' && !ISDIRDELIM(*path)) {
-				path = origpath;
-				back = 0;
-				break;
-			}
-			while (ISDIRDELIM(*path)) path++;
-			origpath = path;
-		}
-
-		while (back--) {
-			/* Strip off path component */
-			while (!ISDIRDELIM(*tmpstr)) {
-				tmpstr--;
-			}
-			if (tmpstr == file_cwd) {
-				/* Incremented again right after the loop. */
-				tmpstr--;
-				break;
-			}
-			/* Skip delimiters */
-			while (ISDIRDELIM(*tmpstr)) tmpstr--;
-		}
-		tmpstr++;
-		if (*path == '\0') {
-			if (tmpstr == file_cwd) {
-				*tmpstr = '/';
-				tmpstr++;
-			}
-			*tmpstr = '\0';
-			return 0;
-		}
-		*tmpstr = '/';
-		pathcpy(tmpstr+1, path);
-	}
-
-	return 0;
-}
-
-int
-file_detectfs(void)
-{
-	int i;
-
-	current_filesystem = FSTYPE_NONE;
-
-	for (i = 0; i < NUM_FILESYS; i++) {
-		if (filesystems[i].detect() == 0) {
-			strcpy(file_cwd, "/");
-			current_filesystem = i;
-			break;
-		}
-	}
-
-	return current_filesystem;
-}
-
-int
-file_ls(const char *dir)
-{
-	char fullpath[1024];
-	const char *arg;
-
-	if (current_filesystem == FSTYPE_NONE) {
-		printf("Can't list files without a filesystem!\n");
-		return -1;
-	}
-
-	if (ISDIRDELIM(*dir)) {
-		arg = dir;
-	} else {
-		sprintf(fullpath, "%s/%s", file_cwd, dir);
-		arg = fullpath;
-	}
-	return filesystems[current_filesystem].ls(arg);
-}
-
-int file_read(const char *filename, void *buffer, int maxsize)
-{
-	char fullpath[1024];
-	const char *arg;
-
-	if (current_filesystem == FSTYPE_NONE) {
-		printf("Can't load file without a filesystem!\n");
-		return -1;
-	}
-
-	if (ISDIRDELIM(*filename)) {
-		arg = filename;
-	} else {
-		sprintf(fullpath, "%s/%s", file_cwd, filename);
-		arg = fullpath;
-	}
-
-	return filesystems[current_filesystem].read(arg, buffer, maxsize);
-}
diff --git a/include/fat.h b/include/fat.h
index 3e7ab9ea8d..1e8bc44e9a 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -171,25 +171,6 @@ typedef struct {
 	int	rootdir_size;
 } fsdata;
 
-typedef int	(file_detectfs_func)(void);
-typedef int	(file_ls_func)(const char *dir);
-typedef int	(file_read_func)(const char *filename, void *buffer,
-				 int maxsize);
-
-struct filesystem {
-	file_detectfs_func	*detect;
-	file_ls_func		*ls;
-	file_read_func		*read;
-	const char		name[12];
-};
-
-/* FAT tables */
-file_detectfs_func	file_fat_detectfs;
-file_ls_func		file_fat_ls;
-file_read_func		file_fat_read;
-
-/* Currently this doesn't check if the dir exists or is valid... */
-int file_cd(const char *path);
 int file_fat_detectfs(void);
 int file_fat_ls(const char *dir);
 int fat_exists(const char *filename);
@@ -197,7 +178,6 @@ int fat_size(const char *filename, loff_t *size);
 int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
 		     loff_t maxsize, loff_t *actread);
 int file_fat_read(const char *filename, void *buffer, int maxsize);
-const char *file_getfsname(int idx);
 int fat_set_blk_dev(struct blk_desc *rbdd, disk_partition_t *info);
 int fat_register_device(struct blk_desc *dev_desc, int part_no);
 
-- 
2.13.5

[U-Boot] [PATCH v2 7/8] fat/fs: move ls to generic implementation

[Rob Clark]

Add a generic implementation of 'ls' using opendir/readdir/closedir, and
replace fat's custom implementation.  Other filesystems should move to
the generic implementation after they add opendir/readdir/closedir
support.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c  | 32 --------------------------------
 fs/fs.c       | 35 +++++++++++++++++++++++++++++++++--
 include/fat.h |  5 ++++-
 3 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index d30ef3903b..fc3106aacb 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -1002,38 +1002,6 @@ int file_fat_detectfs(void)
 	return 0;
 }
 
-int file_fat_ls(const char *dir)
-{
-	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 0;
-}
-
 int fat_exists(const char *filename)
 {
 	fsdata fsdata;
diff --git a/fs/fs.c b/fs/fs.c
index 441c880654..716c223ec6 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -37,6 +37,35 @@ static inline int fs_ls_unsupported(const char *dirname)
 	return -1;
 }
 
+/* generic implementation of ls in terms of opendir/readdir/closedir */
+__maybe_unused
+static int fs_ls_generic(const char *dirname)
+{
+	FS_DIR *dirp;
+	struct fs_dirent *dent;
+	int files = 0, dirs = 0;
+
+	dirp = fs_opendir(dirname);
+	if (!dirp)
+		return -errno;
+
+	while ((dent = fs_readdir(dirp))) {
+		if (dent->type == FS_DT_DIR) {
+			printf("            %s/\n", dent->name);
+			dirs++;
+		} else {
+			printf(" %8lld   %s\n", dent->size, dent->name);
+			files++;
+		}
+	}
+
+	fs_closedir(dirp);
+
+	printf("\n%d file(s), %d dir(s)\n\n", files, dirs);
+
+	return 0;
+}
+
 static inline int fs_exists_unsupported(const char *filename)
 {
 	return 0;
@@ -111,7 +140,7 @@ static struct fstype_info fstypes[] = {
 		.null_dev_desc_ok = false,
 		.probe = fat_set_blk_dev,
 		.close = fat_close,
-		.ls = file_fat_ls,
+		.ls = fs_ls_generic,
 		.exists = fat_exists,
 		.size = fat_size,
 		.read = fat_read_file,
@@ -121,7 +150,9 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_unsupported,
 #endif
 		.uuid = fs_uuid_unsupported,
-		.opendir = fs_opendir_unsupported,
+		.opendir = fat_opendir,
+		.readdir = fat_readdir,
+		.closedir = fat_closedir,
 	},
 #endif
 #ifdef CONFIG_FS_EXT4
diff --git a/include/fat.h b/include/fat.h
index 1e8bc44e9a..b2d4b952fd 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -11,6 +11,7 @@
 #define _FAT_H_
 
 #include <asm/byteorder.h>
+#include <fs.h>
 
 #define CONFIG_SUPPORT_VFAT
 /* Maximum Long File Name length supported here is 128 UTF-16 code units */
@@ -172,7 +173,6 @@ typedef struct {
 } fsdata;
 
 int file_fat_detectfs(void);
-int file_fat_ls(const char *dir);
 int fat_exists(const char *filename);
 int fat_size(const char *filename, loff_t *size);
 int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
@@ -185,5 +185,8 @@ int file_fat_write(const char *filename, void *buf, loff_t offset, loff_t len,
 		   loff_t *actwrite);
 int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
 		  loff_t *actread);
+int fat_opendir(const char *filename, FS_DIR **dirp);
+int fat_readdir(FS_DIR *dirp);
+void fat_closedir(FS_DIR *dirp);
 void fat_close(void);
 #endif /* _FAT_H_ */
-- 
2.13.5

[U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames

[Rob Clark]

Noticed when comparing our output to linux.  There are some lcase bits
which control whether filename and/or extension should be downcase'd.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c       | 17 ++++++++++++-----
 fs/fat/fat_write.c |  4 ++--
 include/fat.h      |  3 +++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index fc3106aacb..ccbf7ba1c8 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -29,11 +29,13 @@ static const int vfat_enabled = 0;
 #endif
 
 /*
- * Convert a string to lowercase.
+ * Convert a string to lowercase.  Converts at most 'len' characters,
+ * 'len' may be larger than the length of 'str' if 'str' is NULL
+ * terminated.
  */
-static void downcase(char *str)
+static void downcase(char *str, size_t len)
 {
-	while (*str != '\0') {
+	while (*str != '\0' && len--) {
 		*str = tolower(*str);
 		str++;
 	}
@@ -131,10 +133,16 @@ static void get_name(dir_entry *dirent, char *s_name)
 	ptr = s_name;
 	while (*ptr && *ptr != ' ')
 		ptr++;
+	if (dirent->lcase & CASE_LOWER_BASE)
+		downcase(s_name, (unsigned)(ptr - s_name));
 	if (dirent->ext[0] && dirent->ext[0] != ' ') {
+		char *ext;
+
 		*ptr = '.';
-		ptr++;
+		ext = ++ptr;
 		memcpy(ptr, dirent->ext, 3);
+		if (dirent->lcase & CASE_LOWER_EXT)
+			downcase(ext, 3);
 		ptr[3] = '\0';
 		while (*ptr && *ptr != ' ')
 			ptr++;
@@ -144,7 +152,6 @@ static void get_name(dir_entry *dirent, char *s_name)
 		*s_name = '\0';
 	else if (*s_name == aRING)
 		*s_name = DELETED_FLAG;
-	downcase(s_name);
 }
 
 static int flush_dirty_fat_buffer(fsdata *mydata);
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 4ca024c208..655ad4ec84 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
 		*l_name = '\0';
 	else if (*l_name == aRING)
 		*l_name = DELETED_FLAG;
-	downcase(l_name);
+	downcase(l_name, ~0);
 
 	/* Return the real directory entry */
 	*retdent = realdent;
@@ -981,7 +981,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 
 	memcpy(l_filename, filename, name_len);
 	l_filename[name_len] = 0; /* terminate the string */
-	downcase(l_filename);
+	downcase(l_filename, ~0);
 
 	startsect = mydata->rootdir_sect;
 	retdent = find_directory_entry(mydata, startsect,
diff --git a/include/fat.h b/include/fat.h
index b2d4b952fd..5e4924316a 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -128,6 +128,9 @@ typedef struct volume_info
 	/* Boot sign comes last, 2 bytes */
 } volume_info;
 
+#define CASE_LOWER_BASE	8	/* base is lower case */
+#define CASE_LOWER_EXT	16	/* extension is lower case */
+
 typedef struct dir_entry {
 	char	name[8],ext[3];	/* Name and extension */
 	__u8	attr;		/* Attribute bits */
-- 
2.13.5

[U-Boot] [PATCH v2 1/8] fs/fat: split out helper to init fsdata

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 4339 bytes --]

On 09/02/2017 06:37 PM, Rob Clark wrote:
> Want to re-use this in fat dirent iterator in next patch.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Reviewed-by: Łukasz Majewski <lukma@denx.de>

> ---
>   fs/fat/fat.c  | 73 +++++++++++++++++++++++++++++++++++------------------------
>   include/fat.h |  1 +
>   2 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 465a6875ed..e1c0a15dc7 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -808,35 +808,17 @@ exit:
>   	return ret;
>   }
>   
> -__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
> -	__aligned(ARCH_DMA_MINALIGN);
> -
> -int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
> -		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
> +static int get_fs_info(fsdata *mydata)
>   {
> -	char fnamecopy[2048];
>   	boot_sector bs;
>   	volume_info volinfo;
> -	fsdata datablock;
> -	fsdata *mydata = &datablock;
> -	dir_entry *dentptr = NULL;
> -	__u16 prevcksum = 0xffff;
> -	char *subname = "";
> -	__u32 cursect;
> -	int idx, isdir = 0;
> -	int files = 0, dirs = 0;
> -	int ret = -1;
> -	int firsttime;
>   	__u32 root_cluster = 0;
> -	__u32 read_blk;
> -	int rootdir_size = 0;
> -	int buffer_blk_cnt;
> -	int do_read;
> -	__u8 *dir_ptr;
> +	int ret;
>   
> -	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
> +	ret = read_bootsectandvi(&bs, &volinfo, &mydata->fatsize);
> +	if (ret) {
>   		debug("Error: reading boot sector\n");
> -		return -1;
> +		return ret;
>   	}
>   
>   	if (mydata->fatsize == 32) {
> @@ -848,8 +830,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
>   
>   	mydata->fat_sect = bs.reserved;
>   
> -	cursect = mydata->rootdir_sect
> -		= mydata->fat_sect + mydata->fatlength * bs.fats;
> +	mydata->rootdir_sect = mydata->fat_sect + mydata->fatlength * bs.fats;
>   
>   	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
>   	mydata->clust_size = bs.cluster_size;
> @@ -863,12 +844,12 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
>   		mydata->data_begin = mydata->rootdir_sect -
>   					(mydata->clust_size * 2);
>   	} else {
> -		rootdir_size = ((bs.dir_entries[1]  * (int)256 +
> -				 bs.dir_entries[0]) *
> -				 sizeof(dir_entry)) /
> -				 mydata->sect_size;
> +		mydata->rootdir_size = ((bs.dir_entries[1]  * (int)256 +
> +					 bs.dir_entries[0]) *
> +					 sizeof(dir_entry)) /
> +					 mydata->sect_size;
>   		mydata->data_begin = mydata->rootdir_sect +
> -					rootdir_size -
> +					mydata->rootdir_size -
>   					(mydata->clust_size * 2);
>   	}
>   
> @@ -893,6 +874,38 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
>   	debug("Sector size: %d, cluster size: %d\n", mydata->sect_size,
>   	      mydata->clust_size);
>   
> +	return 0;
> +}
> +
> +__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
> +	__aligned(ARCH_DMA_MINALIGN);
> +
> +int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
> +		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
> +{
> +	char fnamecopy[2048];
> +	fsdata datablock;
> +	fsdata *mydata = &datablock;
> +	dir_entry *dentptr = NULL;
> +	__u16 prevcksum = 0xffff;
> +	char *subname = "";
> +	__u32 cursect;
> +	int idx, isdir = 0;
> +	int files = 0, dirs = 0;
> +	int ret = -1;
> +	int firsttime;
> +	__u32 root_cluster = 0;
> +	__u32 read_blk;
> +	int rootdir_size = 0;
> +	int buffer_blk_cnt;
> +	int do_read;
> +	__u8 *dir_ptr;
> +
> +	if (get_fs_info(mydata))
> +		return -1;
> +
> +	cursect = mydata->rootdir_sect;
> +
>   	/* "cwd" is always the root... */
>   	while (ISDIRDELIM(*filename))
>   		filename++;
> diff --git a/include/fat.h b/include/fat.h
> index 71879f01ca..6d3fc8e4a6 100644
> --- a/include/fat.h
> +++ b/include/fat.h
> @@ -174,6 +174,7 @@ typedef struct {
>   	__u16	clust_size;	/* Size of clusters in sectors */
>   	int	data_begin;	/* The sector of the first cluster, can be negative */
>   	int	fatbufnum;	/* Used by get_fatent, init to -1 */
> +	int	rootdir_size;
>   } fsdata;
>   
>   typedef int	(file_detectfs_func)(void);
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 9621 bytes --]

On 09/02/2017 06:37 PM, Rob Clark wrote:
> Untangle directory traversal into a simple iterator, to replace the
> existing multi-purpose do_fat_read_at() + get_dentfromdir().
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Reviewed-by: Łukasz Majewski <lukma@denx.de>

> ---
>   fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 326 insertions(+)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index e1c0a15dc7..c72d6ca931 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -1245,6 +1245,332 @@ exit:
>   	return ret;
>   }
>   
> +
> +/*
> + * Directory iterator, to simplify filesystem traversal
> + */
> +
> +typedef struct {
> +	fsdata    *fsdata;
> +	unsigned   cursect;
> +	dir_entry *dent;
> +	int        remaining;     /* remaining dent's in current cluster */
> +	int        last_cluster;
> +	int        is_root;
> +
> +	/* current iterator position values: */
> +	char       l_name[VFAT_MAXLEN_BYTES];
> +	char       s_name[14];
> +	char      *name;          /* l_name if there is one, else s_name */
> +
> +	u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
> +} fat_itr;
> +
> +static int fat_itr_isdir(fat_itr *itr);
> +
> +/**
> + * fat_itr_root() - initialize an iterator to start at the root
> + * directory
> + *
> + * @itr: iterator to initialize
> + * @fsdata: filesystem data for the partition
> + * @return 0 on success, else -errno
> + */
> +static int fat_itr_root(fat_itr *itr, fsdata *fsdata)
> +{
> +	if (get_fs_info(fsdata))
> +		return -ENXIO;
> +
> +	itr->fsdata = fsdata;
> +	itr->cursect = fsdata->rootdir_sect;
> +	itr->dent = NULL;
> +	itr->remaining = 0;
> +	itr->last_cluster = 0;
> +	itr->is_root = 1;
> +
> +	return 0;
> +}
> +
> +/**
> + * fat_itr_child() - initialize an iterator to descend into a sub-
> + * directory
> + *
> + * Initializes 'itr' to iterate the contents of the directory at
> + * the current cursor position of 'parent'.  It is an error to
> + * call this if the current cursor of 'parent' is pointing at a
> + * regular file.
> + *
> + * Note that 'itr' and 'parent' can be the same pointer if you do
> + * not need to preserve 'parent' after this call, which is useful
> + * for traversing directory structure to resolve a file/directory.
> + *
> + * @itr: iterator to initialize
> + * @parent: the iterator pointing at a directory entry in the
> + *    parent directory of the directory to iterate
> + */
> +static void fat_itr_child(fat_itr *itr, fat_itr *parent)
> +{
> +	fsdata *mydata = parent->fsdata;  /* for silly macros */
> +	unsigned clustnum = START(parent->dent);
> +
> +	assert(fat_itr_isdir(parent));
> +
> +	itr->fsdata = parent->fsdata;
> +	if (clustnum > 0) {
> +		itr->cursect = itr->fsdata->data_begin +
> +			(clustnum * itr->fsdata->clust_size);
> +	} else {
> +		itr->cursect = parent->fsdata->rootdir_sect;
> +	}
> +	itr->dent = NULL;
> +	itr->remaining = 0;
> +	itr->last_cluster = 0;
> +	itr->is_root = 0;
> +}
> +
> +static void *next_cluster(fat_itr *itr)
> +{
> +	fsdata *mydata = itr->fsdata;  /* for silly macros */
> +	int ret;
> +
> +	/* have we reached the end? */
> +	if (itr->last_cluster)
> +		return NULL;
> +
> +	debug("FAT read(sect=%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n",
> +	      itr->cursect, itr->fsdata->clust_size, DIRENTSPERBLOCK);
> +
> +	/*
> +	 * NOTE: do_fat_read_at() had complicated logic to deal w/
> +	 * vfat names that span multiple clusters in the fat16 case,
> +	 * which get_dentfromdir() probably also needed (and was
> +	 * missing).  And not entirely sure what fat32 didn't have
> +	 * the same issue..  We solve that by only caring about one
> +	 * dent at a time and iteratively constructing the vfat long
> +	 * name.
> +	 */
> +	ret = disk_read(itr->cursect, itr->fsdata->clust_size,
> +			itr->block);
> +	if (ret < 0) {
> +		debug("Error: reading block\n");
> +		return NULL;
> +	}
> +
> +	if (itr->is_root && itr->fsdata->fatsize != 32) {
> +		itr->cursect++;
> +		if (itr->cursect - itr->fsdata->rootdir_sect >=
> +		    itr->fsdata->rootdir_size) {
> +			debug("cursect: 0x%x\n", itr->cursect);
> +			itr->last_cluster = 1;
> +		}
> +	} else {
> +		itr->cursect = get_fatent(itr->fsdata, itr->cursect);
> +		if (CHECK_CLUST(itr->cursect, itr->fsdata->fatsize)) {
> +			debug("cursect: 0x%x\n", itr->cursect);
> +			itr->last_cluster = 1;
> +		}
> +	}
> +
> +	return itr->block;
> +}
> +
> +static dir_entry *next_dent(fat_itr *itr)
> +{
> +	if (itr->remaining == 0) {
> +		struct dir_entry *dent = next_cluster(itr);
> +
> +		/* have we reached the last cluster? */
> +		if (!dent)
> +			return NULL;
> +
> +		itr->remaining = itr->fsdata->sect_size / sizeof(dir_entry) - 1;
> +		itr->dent = dent;
> +	} else {
> +		itr->remaining--;
> +		itr->dent++;
> +	}
> +
> +	/* have we reached the last valid entry? */
> +	if (itr->dent->name[0] == 0)
> +		return NULL;
> +
> +	return itr->dent;
> +}
> +
> +static dir_entry *extract_vfat_name(fat_itr *itr)
> +{
> +	struct dir_entry *dent = itr->dent;
> +	int seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK;
> +	u8 chksum, alias_checksum = ((dir_slot *)dent)->alias_checksum;
> +	int n = 0;
> +
> +	while (seqn--) {
> +		char buf[13];
> +		int idx = 0;
> +
> +		slot2str((dir_slot *)dent, buf, &idx);
> +
> +		/* shift accumulated long-name up and copy new part in: */
> +		memmove(itr->l_name + idx, itr->l_name, n);
> +		memcpy(itr->l_name, buf, idx);
> +		n += idx;
> +
> +		dent = next_dent(itr);
> +		if (!dent)
> +			return NULL;
> +	}
> +
> +	itr->l_name[n] = '\0';
> +
> +	chksum = mkcksum(dent->name, dent->ext);
> +
> +	/* checksum mismatch could mean deleted file, etc.. skip it: */
> +	if (chksum != alias_checksum) {
> +		debug("** chksum=%x, alias_checksum=%x, l_name=%s, s_name=%8s.%3s\n",
> +		      chksum, alias_checksum, itr->l_name, dent->name, dent->ext);
> +		return NULL;
> +	}
> +
> +	return dent;
> +}
> +
> +/**
> + * fat_itr_next() - step to the next entry in a directory
> + *
> + * Must be called once on a new iterator before the cursor is valid.
> + *
> + * @itr: the iterator to iterate
> + * @return boolean, 1 if success or 0 if no more entries in the
> + *    current directory
> + */
> +static int fat_itr_next(fat_itr *itr)
> +{
> +	dir_entry *dent;
> +
> +	itr->name = NULL;
> +
> +	while (1) {
> +		dent = next_dent(itr);
> +		if (!dent)
> +			return 0;
> +
> +		if (dent->name[0] == DELETED_FLAG ||
> +		    dent->name[0] == aRING)
> +			continue;
> +
> +		if (dent->attr & ATTR_VOLUME) {
> +			if (vfat_enabled &&
> +			    (dent->attr & ATTR_VFAT) == ATTR_VFAT &&
> +			    (dent->name[0] & LAST_LONG_ENTRY_MASK)) {
> +				dent = extract_vfat_name(itr);
> +				if (!dent)
> +					continue;
> +				itr->name = itr->l_name;
> +				break;
> +			} else {
> +				/* Volume label or VFAT entry, skip */
> +				continue;
> +			}
> +		}
> +
> +		break;
> +	}
> +
> +	get_name(dent, itr->s_name);
> +	if (!itr->name)
> +		itr->name = itr->s_name;
> +
> +	return 1;
> +}
> +
> +/**
> + * fat_itr_isdir() - is current cursor position pointing to a directory
> + *
> + * @itr: the iterator
> + * @return true if cursor is at a directory
> + */
> +static int fat_itr_isdir(fat_itr *itr)
> +{
> +	return !!(itr->dent->attr & ATTR_DIR);
> +}
> +
> +/*
> + * Helpers:
> + */
> +
> +#define TYPE_FILE 0x1
> +#define TYPE_DIR  0x2
> +#define TYPE_ANY  (TYPE_FILE | TYPE_DIR)
> +
> +/**
> + * fat_itr_resolve() - traverse directory structure to resolve the
> + * requested path.
> + *
> + * Traverse directory structure to the requested path.  If the specified
> + * path is to a directory, this will descend into the directory and
> + * leave it iterator at the start of the directory.  If the path is to a
> + * file, it will leave the iterator in the parent directory with current
> + * cursor at file's entry in the directory.
> + *
> + * @itr: iterator initialized to root
> + * @path: the requested path
> + * @type: bitmask of allowable file types
> + * @return 0 on success or -errno
> + */
> +static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)
> +{
> +	const char *next;
> +
> +	/* chomp any extra leading slashes: */
> +	while (path[0] && ISDIRDELIM(path[0]))
> +		path++;
> +
> +	/* are we at the end? */
> +	if (strlen(path) == 0) {
> +		if (!(type & TYPE_DIR))
> +			return -ENOENT;
> +		return 0;
> +	}
> +
> +	/* find length of next path entry: */
> +	next = path;
> +	while (next[0] && !ISDIRDELIM(next[0]))
> +		next++;
> +
> +	while (fat_itr_next(itr)) {
> +		int match = 0;
> +
> +		/* check both long and short name: */
> +		if (!strncasecmp(path, itr->name, next - path))
> +			match = 1;
> +		else if (itr->name != itr->s_name &&
> +			 !strncasecmp(path, itr->s_name, (next - path)))
> +			match = 1;
> +
> +		if (!match)
> +			continue;
> +
> +		if (fat_itr_isdir(itr)) {
> +			/* recurse into directory: */
> +			fat_itr_child(itr, itr);
> +			return fat_itr_resolve(itr, next, type);
> +		} else if (next[0]) {
> +			/*
> +			 * If next is not empty then we have a case
> +			 * like: /path/to/realfile/nonsense
> +			 */
> +			debug("bad trailing path: %s\n", next);
> +			return -ENOENT;
> +		} else if (!(type & TYPE_FILE)) {
> +			return -ENOTDIR;
> +		} else {
> +			return 0;
> +		}
> +	}
> +
> +	return -ENOENT;
> +}
> +
>   int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int dols,
>   		loff_t *actread)
>   {
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 21269 bytes --]

On 09/02/2017 06:37 PM, Rob Clark wrote:
> And drop a whole lot of ugly code!

+1

Reviewed-by: Łukasz Majewski <lukma@denx.de>


> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   fs/fat/fat.c  | 723 ++++++----------------------------------------------------
>   include/fat.h |   6 -
>   2 files changed, 75 insertions(+), 654 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index c72d6ca931..3193290434 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)
>   }
>   
>   /*
> - * Get the first occurence of a directory delimiter ('/' or '\') in a string.
> - * Return index into string if found, -1 otherwise.
> - */
> -static int dirdelim(char *str)
> -{
> -	char *start = str;
> -
> -	while (*str != '\0') {
> -		if (ISDIRDELIM(*str))
> -			return str - start;
> -		str++;
> -	}
> -	return -1;
> -}
> -
> -/*
>    * 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;
>   }
>   
> -/*
> - * Extract the full long filename starting at 'retdent' (which is really
> - * a slot) into 'l_name'. If successful also copy the real directory entry
> - * into 'retdent'
> - * Return 0 on success, -1 otherwise.
> - */
> -static int
> -get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
> -	     dir_entry *retdent, char *l_name)
> -{
> -	dir_entry *realdent;
> -	dir_slot *slotptr = (dir_slot *)retdent;
> -	__u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ?
> -							PREFETCH_BLOCKS :
> -							mydata->clust_size);
> -	__u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
> -	int idx = 0;
> -
> -	if (counter > VFAT_MAXSEQ) {
> -		debug("Error: VFAT name is too long\n");
> -		return -1;
> -	}
> -
> -	while ((__u8 *)slotptr < buflimit) {
> -		if (counter == 0)
> -			break;
> -		if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) != counter)
> -			return -1;
> -		slotptr++;
> -		counter--;
> -	}
> -
> -	if ((__u8 *)slotptr >= buflimit) {
> -		dir_slot *slotptr2;
> -
> -		if (curclust == 0)
> -			return -1;
> -		curclust = get_fatent(mydata, curclust);
> -		if (CHECK_CLUST(curclust, mydata->fatsize)) {
> -			debug("curclust: 0x%x\n", curclust);
> -			printf("Invalid FAT entry\n");
> -			return -1;
> -		}
> -
> -		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
> -				mydata->clust_size * mydata->sect_size) != 0) {
> -			debug("Error: reading directory block\n");
> -			return -1;
> -		}
> -
> -		slotptr2 = (dir_slot *)get_contents_vfatname_block;
> -		while (counter > 0) {
> -			if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
> -			    & 0xff) != counter)
> -				return -1;
> -			slotptr2++;
> -			counter--;
> -		}
> -
> -		/* Save the real directory entry */
> -		realdent = (dir_entry *)slotptr2;
> -		while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
> -			slotptr2--;
> -			slot2str(slotptr2, l_name, &idx);
> -		}
> -	} else {
> -		/* Save the real directory entry */
> -		realdent = (dir_entry *)slotptr;
> -	}
> -
> -	do {
> -		slotptr--;
> -		if (slot2str(slotptr, l_name, &idx))
> -			break;
> -	} while (!(slotptr->id & LAST_LONG_ENTRY_MASK));
> -
> -	l_name[idx] = '\0';
> -	if (*l_name == DELETED_FLAG)
> -		*l_name = '\0';
> -	else if (*l_name == aRING)
> -		*l_name = DELETED_FLAG;
> -	downcase(l_name);
> -
> -	/* Return the real directory entry */
> -	memcpy(retdent, realdent, sizeof(dir_entry));
> -
> -	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;
>   }
>   
> -/*
> - * Get the directory entry associated with 'filename' from the directory
> - * starting at 'startsect'
> - */
> +// These should probably DIAF..
>   __u8 get_dentfromdir_block[MAX_CLUSTSIZE]
>   	__aligned(ARCH_DMA_MINALIGN);
> -
> -static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
> -				  char *filename, dir_entry *retdent,
> -				  int dols)
> -{
> -	__u16 prevcksum = 0xffff;
> -	__u32 curclust = START(retdent);
> -	int files = 0, dirs = 0;
> -
> -	debug("get_dentfromdir: %s\n", filename);
> -
> -	while (1) {
> -		dir_entry *dentptr;
> -
> -		int i;
> -
> -		if (get_cluster(mydata, curclust, get_dentfromdir_block,
> -				mydata->clust_size * mydata->sect_size) != 0) {
> -			debug("Error: reading directory block\n");
> -			return NULL;
> -		}
> -
> -		dentptr = (dir_entry *)get_dentfromdir_block;
> -
> -		for (i = 0; i < DIRENTSPERCLUST; i++) {
> -			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
> -
> -			l_name[0] = '\0';
> -			if (dentptr->name[0] == DELETED_FLAG) {
> -				dentptr++;
> -				continue;
> -			}
> -			if ((dentptr->attr & ATTR_VOLUME)) {
> -				if (vfat_enabled &&
> -				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
> -				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
> -					prevcksum = ((dir_slot *)dentptr)->alias_checksum;
> -					get_vfatname(mydata, curclust,
> -						     get_dentfromdir_block,
> -						     dentptr, l_name);
> -					if (dols) {
> -						int isdir;
> -						char dirc;
> -						int doit = 0;
> -
> -						isdir = (dentptr->attr & ATTR_DIR);
> -
> -						if (isdir) {
> -							dirs++;
> -							dirc = '/';
> -							doit = 1;
> -						} else {
> -							dirc = ' ';
> -							if (l_name[0] != 0) {
> -								files++;
> -								doit = 1;
> -							}
> -						}
> -						if (doit) {
> -							if (dirc == ' ') {
> -								printf(" %8u   %s%c\n",
> -								       FAT2CPU32(dentptr->size),
> -									l_name,
> -									dirc);
> -							} else {
> -								printf("            %s%c\n",
> -									l_name,
> -									dirc);
> -							}
> -						}
> -						dentptr++;
> -						continue;
> -					}
> -					debug("vfatname: |%s|\n", l_name);
> -				} else {
> -					/* Volume label or VFAT entry */
> -					dentptr++;
> -					continue;
> -				}
> -			}
> -			if (dentptr->name[0] == 0) {
> -				if (dols) {
> -					printf("\n%d file(s), %d dir(s)\n\n",
> -						files, dirs);
> -				}
> -				debug("Dentname == NULL - %d\n", i);
> -				return NULL;
> -			}
> -			if (vfat_enabled) {
> -				__u8 csum = mkcksum(dentptr->name, dentptr->ext);
> -				if (dols && csum == prevcksum) {
> -					prevcksum = 0xffff;
> -					dentptr++;
> -					continue;
> -				}
> -			}
> -
> -			get_name(dentptr, s_name);
> -			if (dols) {
> -				int isdir = (dentptr->attr & ATTR_DIR);
> -				char dirc;
> -				int doit = 0;
> -
> -				if (isdir) {
> -					dirs++;
> -					dirc = '/';
> -					doit = 1;
> -				} else {
> -					dirc = ' ';
> -					if (s_name[0] != 0) {
> -						files++;
> -						doit = 1;
> -					}
> -				}
> -
> -				if (doit) {
> -					if (dirc == ' ') {
> -						printf(" %8u   %s%c\n",
> -						       FAT2CPU32(dentptr->size),
> -							s_name, dirc);
> -					} else {
> -						printf("            %s%c\n",
> -							s_name, dirc);
> -					}
> -				}
> -
> -				dentptr++;
> -				continue;
> -			}
> -
> -			if (strcmp(filename, s_name)
> -			    && strcmp(filename, l_name)) {
> -				debug("Mismatch: |%s|%s|\n", s_name, l_name);
> -				dentptr++;
> -				continue;
> -			}
> -
> -			memcpy(retdent, dentptr, sizeof(dir_entry));
> -
> -			debug("DentName: %s", s_name);
> -			debug(", start: 0x%x", START(dentptr));
> -			debug(", size:  0x%x %s\n",
> -			      FAT2CPU32(dentptr->size),
> -			      (dentptr->attr & ATTR_DIR) ? "(DIR)" : "");
> -
> -			return retdent;
> -		}
> -
> -		curclust = get_fatent(mydata, curclust);
> -		if (CHECK_CLUST(curclust, mydata->fatsize)) {
> -			debug("curclust: 0x%x\n", curclust);
> -			printf("Invalid FAT entry\n");
> -			return NULL;
> -		}
> -	}
> -
> -	return NULL;
> -}
> +__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
> +	__aligned(ARCH_DMA_MINALIGN);
>   
>   /*
>    * Read boot sector and volume info from a FAT filesystem
> @@ -877,374 +613,6 @@ static int get_fs_info(fsdata *mydata)
>   	return 0;
>   }
>   
> -__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
> -	__aligned(ARCH_DMA_MINALIGN);
> -
> -int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
> -		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
> -{
> -	char fnamecopy[2048];
> -	fsdata datablock;
> -	fsdata *mydata = &datablock;
> -	dir_entry *dentptr = NULL;
> -	__u16 prevcksum = 0xffff;
> -	char *subname = "";
> -	__u32 cursect;
> -	int idx, isdir = 0;
> -	int files = 0, dirs = 0;
> -	int ret = -1;
> -	int firsttime;
> -	__u32 root_cluster = 0;
> -	__u32 read_blk;
> -	int rootdir_size = 0;
> -	int buffer_blk_cnt;
> -	int do_read;
> -	__u8 *dir_ptr;
> -
> -	if (get_fs_info(mydata))
> -		return -1;
> -
> -	cursect = mydata->rootdir_sect;
> -
> -	/* "cwd" is always the root... */
> -	while (ISDIRDELIM(*filename))
> -		filename++;
> -
> -	/* Make a copy of the filename and convert it to lowercase */
> -	strcpy(fnamecopy, filename);
> -	downcase(fnamecopy);
> -
> -root_reparse:
> -	if (*fnamecopy == '\0') {
> -		if (!dols)
> -			goto exit;
> -
> -		dols = LS_ROOT;
> -	} else if ((idx = dirdelim(fnamecopy)) >= 0) {
> -		isdir = 1;
> -		fnamecopy[idx] = '\0';
> -		subname = fnamecopy + idx + 1;
> -
> -		/* Handle multiple delimiters */
> -		while (ISDIRDELIM(*subname))
> -			subname++;
> -	} else if (dols) {
> -		isdir = 1;
> -	}
> -
> -	buffer_blk_cnt = 0;
> -	firsttime = 1;
> -	while (1) {
> -		int i;
> -
> -		if (mydata->fatsize == 32 || firsttime) {
> -			dir_ptr = do_fat_read_at_block;
> -			firsttime = 0;
> -		} else {
> -			/**
> -			 * FAT16 sector buffer modification:
> -			 * Each loop, the second buffered block is moved to
> -			 * the buffer begin, and two next sectors are read
> -			 * next to the previously moved one. So the sector
> -			 * buffer keeps always 3 sectors for fat16.
> -			 * And the current sector is the buffer second sector
> -			 * beside the "firsttime" read, when it is the first one.
> -			 *
> -			 * PREFETCH_BLOCKS is 2 for FAT16 == loop[0:1]
> -			 * n = computed root dir sector
> -			 * loop |  cursect-1  | cursect    | cursect+1  |
> -			 *   0  |  sector n+0 | sector n+1 | none       |
> -			 *   1  |  none       | sector n+0 | sector n+1 |
> -			 *   0  |  sector n+1 | sector n+2 | sector n+3 |
> -			 *   1  |  sector n+3 | ...
> -			*/
> -			dir_ptr = (do_fat_read_at_block + mydata->sect_size);
> -			memcpy(do_fat_read_at_block, dir_ptr, mydata->sect_size);
> -		}
> -
> -		do_read = 1;
> -
> -		if (mydata->fatsize == 32 && buffer_blk_cnt)
> -			do_read = 0;
> -
> -		if (do_read) {
> -			read_blk = (mydata->fatsize == 32) ?
> -				    mydata->clust_size : PREFETCH_BLOCKS;
> -
> -			debug("FAT read(sect=%d, cnt:%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n",
> -				cursect, read_blk, mydata->clust_size, DIRENTSPERBLOCK);
> -
> -			if (disk_read(cursect, read_blk, dir_ptr) < 0) {
> -				debug("Error: reading rootdir block\n");
> -				goto exit;
> -			}
> -
> -			dentptr = (dir_entry *)dir_ptr;
> -		}
> -
> -		for (i = 0; i < DIRENTSPERBLOCK; i++) {
> -			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
> -			__u8 csum;
> -
> -			l_name[0] = '\0';
> -			if (dentptr->name[0] == DELETED_FLAG) {
> -				dentptr++;
> -				continue;
> -			}
> -
> -			if (vfat_enabled)
> -				csum = mkcksum(dentptr->name, dentptr->ext);
> -
> -			if (dentptr->attr & ATTR_VOLUME) {
> -				if (vfat_enabled &&
> -				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
> -				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
> -					prevcksum =
> -						((dir_slot *)dentptr)->alias_checksum;
> -
> -					get_vfatname(mydata,
> -						     root_cluster,
> -						     dir_ptr,
> -						     dentptr, l_name);
> -
> -					if (dols == LS_ROOT) {
> -						char dirc;
> -						int doit = 0;
> -						int isdir =
> -							(dentptr->attr & ATTR_DIR);
> -
> -						if (isdir) {
> -							dirs++;
> -							dirc = '/';
> -							doit = 1;
> -						} else {
> -							dirc = ' ';
> -							if (l_name[0] != 0) {
> -								files++;
> -								doit = 1;
> -							}
> -						}
> -						if (doit) {
> -							if (dirc == ' ') {
> -								printf(" %8u   %s%c\n",
> -								       FAT2CPU32(dentptr->size),
> -									l_name,
> -									dirc);
> -							} else {
> -								printf("            %s%c\n",
> -									l_name,
> -									dirc);
> -							}
> -						}
> -						dentptr++;
> -						continue;
> -					}
> -					debug("Rootvfatname: |%s|\n",
> -					       l_name);
> -				} else {
> -					/* Volume label or VFAT entry */
> -					dentptr++;
> -					continue;
> -				}
> -			} else if (dentptr->name[0] == 0) {
> -				debug("RootDentname == NULL - %d\n", i);
> -				if (dols == LS_ROOT) {
> -					printf("\n%d file(s), %d dir(s)\n\n",
> -						files, dirs);
> -					ret = 0;
> -				}
> -				goto exit;
> -			}
> -			else if (vfat_enabled &&
> -				 dols == LS_ROOT && csum == prevcksum) {
> -				prevcksum = 0xffff;
> -				dentptr++;
> -				continue;
> -			}
> -
> -			get_name(dentptr, s_name);
> -
> -			if (dols == LS_ROOT) {
> -				int isdir = (dentptr->attr & ATTR_DIR);
> -				char dirc;
> -				int doit = 0;
> -
> -				if (isdir) {
> -					dirc = '/';
> -					if (s_name[0] != 0) {
> -						dirs++;
> -						doit = 1;
> -					}
> -				} else {
> -					dirc = ' ';
> -					if (s_name[0] != 0) {
> -						files++;
> -						doit = 1;
> -					}
> -				}
> -				if (doit) {
> -					if (dirc == ' ') {
> -						printf(" %8u   %s%c\n",
> -						       FAT2CPU32(dentptr->size),
> -							s_name, dirc);
> -					} else {
> -						printf("            %s%c\n",
> -							s_name, dirc);
> -					}
> -				}
> -				dentptr++;
> -				continue;
> -			}
> -
> -			if (strcmp(fnamecopy, s_name)
> -			    && strcmp(fnamecopy, l_name)) {
> -				debug("RootMismatch: |%s|%s|\n", s_name,
> -				       l_name);
> -				dentptr++;
> -				continue;
> -			}
> -
> -			if (isdir && !(dentptr->attr & ATTR_DIR))
> -				goto exit;
> -
> -			debug("RootName: %s", s_name);
> -			debug(", start: 0x%x", START(dentptr));
> -			debug(", size:  0x%x %s\n",
> -			       FAT2CPU32(dentptr->size),
> -			       isdir ? "(DIR)" : "");
> -
> -			goto rootdir_done;	/* We got a match */
> -		}
> -		debug("END LOOP: buffer_blk_cnt=%d   clust_size=%d\n", buffer_blk_cnt,
> -		       mydata->clust_size);
> -
> -		/*
> -		 * On FAT32 we must fetch the FAT entries for the next
> -		 * root directory clusters when a cluster has been
> -		 * completely processed.
> -		 */
> -		++buffer_blk_cnt;
> -		int rootdir_end = 0;
> -		if (mydata->fatsize == 32) {
> -			if (buffer_blk_cnt == mydata->clust_size) {
> -				int nxtsect = 0;
> -				int nxt_clust = 0;
> -
> -				nxt_clust = get_fatent(mydata, root_cluster);
> -				rootdir_end = CHECK_CLUST(nxt_clust, 32);
> -
> -				nxtsect = mydata->data_begin +
> -					(nxt_clust * mydata->clust_size);
> -
> -				root_cluster = nxt_clust;
> -
> -				cursect = nxtsect;
> -				buffer_blk_cnt = 0;
> -			}
> -		} else {
> -			if (buffer_blk_cnt == PREFETCH_BLOCKS)
> -				buffer_blk_cnt = 0;
> -
> -			rootdir_end = (++cursect - mydata->rootdir_sect >=
> -				       rootdir_size);
> -		}
> -
> -		/* If end of rootdir reached */
> -		if (rootdir_end) {
> -			if (dols == LS_ROOT) {
> -				printf("\n%d file(s), %d dir(s)\n\n",
> -				       files, dirs);
> -				*size = 0;
> -			}
> -			goto exit;
> -		}
> -	}
> -rootdir_done:
> -
> -	firsttime = 1;
> -
> -	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.
> -		 */
> -		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;
> +		}
> +		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)
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 4/8] fs: add fs_readdir()

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 9885 bytes --]

On 09/02/2017 06:37 PM, Rob Clark wrote:
> Needed to support efi file protocol.  The fallback.efi loader wants
> to be able to read the contents of the /EFI directory to find an OS
> to boot.
> 
> Modelled after POSIX opendir()/readdir()/closedir().  Unlike the other
> fs APIs, this is stateful (ie. state is held in the FS_DIR "directory
> stream"), to avoid re-traversing of the directory structure at each
> step.  The directory stream must be released with closedir() when it
> is no longer needed.
> 

Reviewed-by: Łukasz Majewski <lukma@denx.de>

> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   disk/part.c    | 31 ++++++++++++--------
>   fs/fs.c        | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   include/fs.h   | 55 +++++++++++++++++++++++++++++++++++
>   include/part.h |  4 +++
>   4 files changed, 169 insertions(+), 12 deletions(-)
> 
> diff --git a/disk/part.c b/disk/part.c
> index c67fdacc79..aa9183d696 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int part,
>   	return -1;
>   }
>   
> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info)
> +{
> +	info->start = 0;
> +	info->size = dev_desc->lba;
> +	info->blksz = dev_desc->blksz;
> +	info->bootable = 0;
> +	strcpy((char *)info->type, BOOT_PART_TYPE);
> +	strcpy((char *)info->name, "Whole Disk");
> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
> +	info->uuid[0] = 0;
> +#endif
> +#ifdef CONFIG_PARTITION_TYPE_GUID
> +	info->type_guid[0] = 0;
> +#endif
> +
> +	return 0;
> +}
> +
>   int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str,
>   			  struct blk_desc **dev_desc)
>   {
> @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
>   
>   		(*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);
>   
> -		info->start = 0;
> -		info->size = (*dev_desc)->lba;
> -		info->blksz = (*dev_desc)->blksz;
> -		info->bootable = 0;
> -		strcpy((char *)info->type, BOOT_PART_TYPE);
> -		strcpy((char *)info->name, "Whole Disk");
> -#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
> -		info->uuid[0] = 0;
> -#endif
> -#ifdef CONFIG_PARTITION_TYPE_GUID
> -		info->type_guid[0] = 0;
> -#endif
> +		part_get_info_whole_disk(*dev_desc, info);
>   
>   		ret = 0;
>   		goto cleanup;
> diff --git a/fs/fs.c b/fs/fs.c
> index 13cd3626c6..441c880654 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -21,6 +21,7 @@
>   DECLARE_GLOBAL_DATA_PTR;
>   
>   static struct blk_desc *fs_dev_desc;
> +static int fs_dev_part;
>   static disk_partition_t fs_partition;
>   static int fs_type = FS_TYPE_ANY;
>   
> @@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)
>   	return -1;
>   }
>   
> +static inline int fs_opendir_unsupported(const char *filename, FS_DIR **dirp)
> +{
> +	return -EACCES;
> +}
> +
>   struct fstype_info {
>   	int fstype;
>   	char *name;
> @@ -92,6 +98,9 @@ struct fstype_info {
>   		     loff_t len, loff_t *actwrite);
>   	void (*close)(void);
>   	int (*uuid)(char *uuid_str);
> +	int (*opendir)(const char *filename, FS_DIR **dirp);
> +	int (*readdir)(FS_DIR *dirp);
> +	void (*closedir)(FS_DIR *dirp);
>   };
>   
>   static struct fstype_info fstypes[] = {
> @@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {
>   		.write = fs_write_unsupported,
>   #endif
>   		.uuid = fs_uuid_unsupported,
> +		.opendir = fs_opendir_unsupported,
>   	},
>   #endif
>   #ifdef CONFIG_FS_EXT4
> @@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {
>   		.write = fs_write_unsupported,
>   #endif
>   		.uuid = ext4fs_uuid,
> +		.opendir = fs_opendir_unsupported,
>   	},
>   #endif
>   #ifdef CONFIG_SANDBOX
> @@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {
>   		.read = fs_read_sandbox,
>   		.write = fs_write_sandbox,
>   		.uuid = fs_uuid_unsupported,
> +		.opendir = fs_opendir_unsupported,
>   	},
>   #endif
>   #ifdef CONFIG_CMD_UBIFS
> @@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {
>   		.read = ubifs_read,
>   		.write = fs_write_unsupported,
>   		.uuid = fs_uuid_unsupported,
> +		.opendir = fs_opendir_unsupported,
>   	},
>   #endif
>   	{
> @@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {
>   		.read = fs_read_unsupported,
>   		.write = fs_write_unsupported,
>   		.uuid = fs_uuid_unsupported,
> +		.opendir = fs_opendir_unsupported,
>   	},
>   };
>   
> @@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
>   
>   		if (!info->probe(fs_dev_desc, &fs_partition)) {
>   			fs_type = info->fstype;
> +			fs_dev_part = part;
> +			return 0;
> +		}
> +	}
> +
> +	return -1;
> +}
> +
> +/* set current blk device w/ blk_desc + partition # */
> +int fs_set_blk_dev2(struct blk_desc *desc, int part)
> +{
> +	struct fstype_info *info;
> +	int ret, i;
> +
> +	if (part >= 1)
> +		ret = part_get_info(desc, part, &fs_partition);
> +	else
> +		ret = part_get_info_whole_disk(desc, &fs_partition);
> +	if (ret)
> +		return ret;
> +	fs_dev_desc = desc;
> +
> +	for (i = 0, info = fstypes; i < ARRAY_SIZE(fstypes); i++, info++) {
> +		if (!info->probe(fs_dev_desc, &fs_partition)) {
> +			fs_type = info->fstype;
>   			return 0;
>   		}
>   	}
> @@ -334,6 +373,58 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
>   	return ret;
>   }
>   
> +FS_DIR *fs_opendir(const char *filename)
> +{
> +	struct fstype_info *info = fs_get_info(fs_type);
> +	FS_DIR *dirp = NULL;
> +	int ret;
> +
> +	ret = info->opendir(filename, &dirp);
> +	fs_close();
> +	if (ret) {
> +		errno = -ret;
> +		return NULL;
> +	}
> +
> +	dirp->desc = fs_dev_desc;
> +	dirp->part = fs_dev_part;
> +
> +	return dirp;
> +}
> +
> +struct fs_dirent *fs_readdir(FS_DIR *dirp)
> +{
> +	struct fstype_info *info;
> +	int ret;
> +
> +	fs_set_blk_dev2(dirp->desc, dirp->part);
> +	info = fs_get_info(fs_type);
> +
> +	memset(&dirp->dirent, 0, sizeof(dirp->dirent));
> +
> +	ret = info->readdir(dirp);
> +	fs_close();
> +	if (ret)
> +		return NULL;
> +
> +	return &dirp->dirent;;
> +}
> +
> +void fs_closedir(FS_DIR *dirp)
> +{
> +	struct fstype_info *info;
> +
> +	if (!dirp)
> +		return;
> +
> +	fs_set_blk_dev2(dirp->desc, dirp->part);
> +	info = fs_get_info(fs_type);
> +
> +	info->closedir(dirp);
> +	fs_close();
> +}
> +
> +
>   int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>   		int fstype)
>   {
> diff --git a/include/fs.h b/include/fs.h
> index 2f2aca8378..0a6a366078 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -26,6 +26,8 @@
>    */
>   int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);
>   
> +int fs_set_blk_dev2(struct blk_desc *desc, int part);
> +
>   /*
>    * Print the list of files on the partition previously set by fs_set_blk_dev(),
>    * in directory "dirname".
> @@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
>   int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
>   	     loff_t *actwrite);
>   
> +/* Add additional FS_DT_* as supported by additional filesystems:*/
> +#define FS_DT_DIR  0x4       /* directory */
> +#define FS_DT_REG  0x8       /* regular file */
> +
> +/*
> + * A directory entry.
> + */
> +struct fs_dirent {
> +	unsigned type;       /* one of FS_DT_* */
> +	loff_t size;
> +	char name[256];
> +};
> +
> +typedef struct _FS_DIR FS_DIR;
> +
> +/*
> + * fs_opendir - Open a directory
> + *
> + * @filename: the path to directory to open
> + * @return a pointer to the directory stream or NULL on error and errno
> + *    set appropriately
> + */
> +FS_DIR *fs_opendir(const char *filename);
> +
> +/*
> + * fs_readdir - Read the next directory entry in the directory stream.
> + *
> + * @dirp: the directory stream
> + * @return the next directory entry (only valid until next fs_readdir() or
> + *    fs_closedir() call, do not attempt to free()) or NULL if the end of
> + *    the directory is reached.
> + */
> +struct fs_dirent *fs_readdir(FS_DIR *dirp);
> +
> +/*
> + * fs_closedir - close a directory stream
> + *
> + * @dirp: the directory stream
> + */
> +void fs_closedir(FS_DIR *dirp);
> +
> +/*
> + * private to fs implementations, would be in fs.c but we need to let
> + * implementations subclass:
> + */
> +
> +struct _FS_DIR {
> +	struct fs_dirent dirent;
> +	/* private to fs layer: */
> +	struct blk_desc *desc;
> +	int part;
> +};
> +
>   /*
>    * Common implementation for various filesystem commands, optionally limited
>    * to a specific filesystem type via the fstype parameter.
> diff --git a/include/part.h b/include/part.h
> index 0cd803a933..48e8ff6d8a 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc **blk_devp);
>   
>   /* disk/part.c */
>   int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info);
> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info);
>   void part_print(struct blk_desc *dev_desc);
>   void part_init(struct blk_desc *dev_desc);
>   void dev_print(struct blk_desc *dev_desc);
> @@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int dev) { return NULL; }
>   
>   static inline int part_get_info(struct blk_desc *dev_desc, int part,
>   				disk_partition_t *info) { return -1; }
> +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,
> +					   disk_partition_t *info)
> +{ return -1; }
>   static inline void part_print(struct blk_desc *dev_desc) {}
>   static inline void part_init(struct blk_desc *dev_desc) {}
>   static inline void dev_print(struct blk_desc *dev_desc) {}
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 5/8] fs/fat: implement opendir/readdir/closedir

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2094 bytes --]

On 09/02/2017 06:38 PM, Rob Clark wrote:
> Implement the readdir interface using the directory iterators.
> 

Reviewed-by: Łukasz Majewski <lukma@denx.de>

> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   fs/fat/fat.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 56 insertions(+)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 3193290434..d30ef3903b 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -14,6 +14,7 @@
>   #include <config.h>
>   #include <exports.h>
>   #include <fat.h>
> +#include <fs.h>
>   #include <asm/byteorder.h>
>   #include <part.h>
>   #include <malloc.h>
> @@ -1119,6 +1120,61 @@ int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
>   	return ret;
>   }
>   
> +typedef struct {
> +	FS_DIR parent;
> +	fsdata fsdata;
> +	fat_itr itr;
> +} fat_dir;
> +
> +int fat_opendir(const char *filename, FS_DIR **dirp)
> +{
> +	fat_dir *dir = malloc(sizeof(*dir));
> +	int ret;
> +
> +	if (!dir)
> +		return -ENOMEM;
> +
> +	ret = fat_itr_root(&dir->itr, &dir->fsdata);
> +	if (ret)
> +		goto fail;
> +
> +	ret = fat_itr_resolve(&dir->itr, filename, TYPE_DIR);
> +	if (ret)
> +		goto fail;
> +
> +	*dirp = (FS_DIR *)dir;
> +	return 0;
> +
> +fail:
> +	free(dir);
> +	return ret;
> +}
> +
> +int fat_readdir(FS_DIR *dirp)
> +{
> +	fat_dir *dir = (fat_dir *)dirp;
> +	struct fs_dirent *dent = &dirp->dirent;
> +
> +	if (!fat_itr_next(&dir->itr))
> +		return -ENOENT;
> +
> +	strcpy(dent->name, dir->itr.name);
> +	if (fat_itr_isdir(&dir->itr)) {
> +		dent->type = FS_DT_DIR;
> +	} else {
> +		dent->type = FS_DT_REG;
> +		dent->size = FAT2CPU32(dir->itr.dent->size);
> +	}
> +
> +	return 0;
> +}
> +
> +void fat_closedir(FS_DIR *dirp)
> +{
> +	fat_dir *dir = (fat_dir *)dirp;
> +	free(dir);
> +}
> +
>   void fat_close(void)
>   {
>   }
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 7/8] fat/fs: move ls to generic implementation

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 4343 bytes --]

On 09/02/2017 06:38 PM, Rob Clark wrote:
> Add a generic implementation of 'ls' using opendir/readdir/closedir, and
> replace fat's custom implementation.  Other filesystems should move to
> the generic implementation after they add opendir/readdir/closedir
> support.
> 

Reviewed-by: Łukasz Majewski <lukma@denx.de>

> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   fs/fat/fat.c  | 32 --------------------------------
>   fs/fs.c       | 35 +++++++++++++++++++++++++++++++++--
>   include/fat.h |  5 ++++-
>   3 files changed, 37 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index d30ef3903b..fc3106aacb 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -1002,38 +1002,6 @@ int file_fat_detectfs(void)
>   	return 0;
>   }
>   
> -int file_fat_ls(const char *dir)
> -{
> -	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 0;
> -}
> -
>   int fat_exists(const char *filename)
>   {
>   	fsdata fsdata;
> diff --git a/fs/fs.c b/fs/fs.c
> index 441c880654..716c223ec6 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -37,6 +37,35 @@ static inline int fs_ls_unsupported(const char *dirname)
>   	return -1;
>   }
>   
> +/* generic implementation of ls in terms of opendir/readdir/closedir */
> +__maybe_unused
> +static int fs_ls_generic(const char *dirname)
> +{
> +	FS_DIR *dirp;
> +	struct fs_dirent *dent;
> +	int files = 0, dirs = 0;
> +
> +	dirp = fs_opendir(dirname);
> +	if (!dirp)
> +		return -errno;
> +
> +	while ((dent = fs_readdir(dirp))) {
> +		if (dent->type == FS_DT_DIR) {
> +			printf("            %s/\n", dent->name);
> +			dirs++;
> +		} else {
> +			printf(" %8lld   %s\n", dent->size, dent->name);
> +			files++;
> +		}
> +	}
> +
> +	fs_closedir(dirp);
> +
> +	printf("\n%d file(s), %d dir(s)\n\n", files, dirs);
> +
> +	return 0;
> +}
> +
>   static inline int fs_exists_unsupported(const char *filename)
>   {
>   	return 0;
> @@ -111,7 +140,7 @@ static struct fstype_info fstypes[] = {
>   		.null_dev_desc_ok = false,
>   		.probe = fat_set_blk_dev,
>   		.close = fat_close,
> -		.ls = file_fat_ls,
> +		.ls = fs_ls_generic,
>   		.exists = fat_exists,
>   		.size = fat_size,
>   		.read = fat_read_file,
> @@ -121,7 +150,9 @@ static struct fstype_info fstypes[] = {
>   		.write = fs_write_unsupported,
>   #endif
>   		.uuid = fs_uuid_unsupported,
> -		.opendir = fs_opendir_unsupported,
> +		.opendir = fat_opendir,
> +		.readdir = fat_readdir,
> +		.closedir = fat_closedir,
>   	},
>   #endif
>   #ifdef CONFIG_FS_EXT4
> diff --git a/include/fat.h b/include/fat.h
> index 1e8bc44e9a..b2d4b952fd 100644
> --- a/include/fat.h
> +++ b/include/fat.h
> @@ -11,6 +11,7 @@
>   #define _FAT_H_
>   
>   #include <asm/byteorder.h>
> +#include <fs.h>
>   
>   #define CONFIG_SUPPORT_VFAT
>   /* Maximum Long File Name length supported here is 128 UTF-16 code units */
> @@ -172,7 +173,6 @@ typedef struct {
>   } fsdata;
>   
>   int file_fat_detectfs(void);
> -int file_fat_ls(const char *dir);
>   int fat_exists(const char *filename);
>   int fat_size(const char *filename, loff_t *size);
>   int file_fat_read_at(const char *filename, loff_t pos, void *buffer,
> @@ -185,5 +185,8 @@ int file_fat_write(const char *filename, void *buf, loff_t offset, loff_t len,
>   		   loff_t *actwrite);
>   int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
>   		  loff_t *actread);
> +int fat_opendir(const char *filename, FS_DIR **dirp);
> +int fat_readdir(FS_DIR *dirp);
> +void fat_closedir(FS_DIR *dirp);
>   void fat_close(void);
>   #endif /* _FAT_H_ */
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames

[Łukasz Majewski]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 3369 bytes --]

On 09/02/2017 06:38 PM, Rob Clark wrote:
> Noticed when comparing our output to linux.  There are some lcase bits
> which control whether filename and/or extension should be downcase'd.

Reviewed-by: Łukasz Majewski <lukma@denx.de>

> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   fs/fat/fat.c       | 17 ++++++++++++-----
>   fs/fat/fat_write.c |  4 ++--
>   include/fat.h      |  3 +++
>   3 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index fc3106aacb..ccbf7ba1c8 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -29,11 +29,13 @@ static const int vfat_enabled = 0;
>   #endif
>   
>   /*
> - * Convert a string to lowercase.
> + * Convert a string to lowercase.  Converts at most 'len' characters,
> + * 'len' may be larger than the length of 'str' if 'str' is NULL
> + * terminated.
>    */
> -static void downcase(char *str)
> +static void downcase(char *str, size_t len)
>   {
> -	while (*str != '\0') {
> +	while (*str != '\0' && len--) {
>   		*str = tolower(*str);
>   		str++;
>   	}
> @@ -131,10 +133,16 @@ static void get_name(dir_entry *dirent, char *s_name)
>   	ptr = s_name;
>   	while (*ptr && *ptr != ' ')
>   		ptr++;
> +	if (dirent->lcase & CASE_LOWER_BASE)
> +		downcase(s_name, (unsigned)(ptr - s_name));
>   	if (dirent->ext[0] && dirent->ext[0] != ' ') {
> +		char *ext;
> +
>   		*ptr = '.';
> -		ptr++;
> +		ext = ++ptr;
>   		memcpy(ptr, dirent->ext, 3);
> +		if (dirent->lcase & CASE_LOWER_EXT)
> +			downcase(ext, 3);
>   		ptr[3] = '\0';
>   		while (*ptr && *ptr != ' ')
>   			ptr++;
> @@ -144,7 +152,6 @@ static void get_name(dir_entry *dirent, char *s_name)
>   		*s_name = '\0';
>   	else if (*s_name == aRING)
>   		*s_name = DELETED_FLAG;
> -	downcase(s_name);
>   }
>   
>   static int flush_dirty_fat_buffer(fsdata *mydata);
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 4ca024c208..655ad4ec84 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
>   		*l_name = '\0';
>   	else if (*l_name == aRING)
>   		*l_name = DELETED_FLAG;
> -	downcase(l_name);
> +	downcase(l_name, ~0);
>   
>   	/* Return the real directory entry */
>   	*retdent = realdent;
> @@ -981,7 +981,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
>   
>   	memcpy(l_filename, filename, name_len);
>   	l_filename[name_len] = 0; /* terminate the string */
> -	downcase(l_filename);
> +	downcase(l_filename, ~0);
>   
>   	startsect = mydata->rootdir_sect;
>   	retdent = find_directory_entry(mydata, startsect,
> diff --git a/include/fat.h b/include/fat.h
> index b2d4b952fd..5e4924316a 100644
> --- a/include/fat.h
> +++ b/include/fat.h
> @@ -128,6 +128,9 @@ typedef struct volume_info
>   	/* Boot sign comes last, 2 bytes */
>   } volume_info;
>   
> +#define CASE_LOWER_BASE	8	/* base is lower case */
> +#define CASE_LOWER_EXT	16	/* extension is lower case */
> +
>   typedef struct dir_entry {
>   	char	name[8],ext[3];	/* Name and extension */
>   	__u8	attr;		/* Attribute bits */
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 1/8] fs/fat: split out helper to init fsdata

[Rob Clark]

On Sun, Sep 3, 2017 at 10:52 AM, Łukasz Majewski <lukma@denx.de> wrote:
> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>
>> Want to re-use this in fat dirent iterator in next patch.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
>
> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>

thanks

btw, is there some way to make get_maintainers.pl to find your
@denx.de address instead of the @samsung address which bounces?
(Sorry, I hadn't realized you had a new addr, so you were missed from
CC on some of the patches, I think.)

BR,
-R

[U-Boot] [PATCH v2 1/8] fs/fat: split out helper to init fsdata

[Łukasz Majewski]

On 09/03/2017 05:47 PM, Rob Clark wrote:
> On Sun, Sep 3, 2017 at 10:52 AM, Łukasz Majewski <lukma@denx.de> wrote:
>> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>>
>>> Want to re-use this in fat dirent iterator in next patch.
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>
>>
>> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>>
> 
> thanks
> 
> btw, is there some way to make get_maintainers.pl to find your
> @denx.de address instead of the @samsung address which bounces?
> (Sorry, I hadn't realized you had a new addr, so you were missed from
> CC on some of the patches, I think.)

I've updated git-mailrc entry (about which I've forgotten).

However, get_maintainer looks for people making changes to the file(s) 
to which patch is applied.
Hence, you see my old address.

> 
> BR,
> -R
> 


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

[U-Boot] [PATCH v2 1/8] fs/fat: split out helper to init fsdata

[Simon Glass]

Hi Rob,

On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:
> Want to re-use this in fat dirent iterator in next patch.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  fs/fat/fat.c  | 73 +++++++++++++++++++++++++++++++++++------------------------
>  include/fat.h |  1 +
>  2 files changed, 44 insertions(+), 30 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

But please add a comment to the new structure memory you added.

[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Simon Glass]

Hi Rob,

On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:
> Untangle directory traversal into a simple iterator, to replace the
> existing multi-purpose do_fat_read_at() + get_dentfromdir().
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 326 insertions(+)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index e1c0a15dc7..c72d6ca931 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -1245,6 +1245,332 @@ exit:
>         return ret;
>  }
>
> +
> +/*
> + * Directory iterator, to simplify filesystem traversal
> + */
> +
> +typedef struct {

Please avoid using a typedef here. It seems like it could just be a struct.

Also pleaee add a comment about what the struct is for

> +       fsdata    *fsdata;
> +       unsigned   cursect;
> +       dir_entry *dent;
> +       int        remaining;     /* remaining dent's in current cluster */
> +       int        last_cluster;
> +       int        is_root;
> +
> +       /* current iterator position values: */
> +       char       l_name[VFAT_MAXLEN_BYTES];
> +       char       s_name[14];
> +       char      *name;          /* l_name if there is one, else s_name */
> +
> +       u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);

Some members are missing comments.

Also I'm not too sure how the alignment works here. I don't see you
creating one of these objects in this patch, but could you add a
comment to the struct about how to create one? Do you use memalign()?

[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Simon Glass]

Hi Rob,

On 3 September 2017 at 23:08, Łukasz Majewski <lukma@denx.de> wrote:
> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>
>> And drop a whole lot of ugly code!

:-)

>
>
> +1
>
> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>
>
>
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>   fs/fat/fat.c  | 723
>> ++++++----------------------------------------------------
>>   include/fat.h |   6 -
>>   2 files changed, 75 insertions(+), 654 deletions(-)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index c72d6ca931..3193290434 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)
>>   }
>>     /*
>> - * Get the first occurence of a directory delimiter ('/' or '\') in a
>> string.
>> - * Return index into string if found, -1 otherwise.
>> - */
>> -static int dirdelim(char *str)
>> -{
>> -       char *start = str;
>> -
>> -       while (*str != '\0') {
>> -               if (ISDIRDELIM(*str))
>> -                       return str - start;
>> -               str++;
>> -       }
>> -       return -1;
>> -}
>> -
>> -/*
>>    * 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;
>>   }
>>   -/*
>> - * Extract the full long filename starting at 'retdent' (which is really
>> - * a slot) into 'l_name'. If successful also copy the real directory
>> entry
>> - * into 'retdent'
>> - * Return 0 on success, -1 otherwise.
>> - */
>> -static int
>> -get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
>> -            dir_entry *retdent, char *l_name)
>> -{
>> -       dir_entry *realdent;
>> -       dir_slot *slotptr = (dir_slot *)retdent;
>> -       __u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ?
>> -                                                       PREFETCH_BLOCKS :
>> -
>> mydata->clust_size);
>> -       __u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
>> -       int idx = 0;
>> -
>> -       if (counter > VFAT_MAXSEQ) {
>> -               debug("Error: VFAT name is too long\n");
>> -               return -1;
>> -       }
>> -
>> -       while ((__u8 *)slotptr < buflimit) {
>> -               if (counter == 0)
>> -                       break;
>> -               if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) !=
>> counter)
>> -                       return -1;
>> -               slotptr++;
>> -               counter--;
>> -       }
>> -
>> -       if ((__u8 *)slotptr >= buflimit) {
>> -               dir_slot *slotptr2;
>> -
>> -               if (curclust == 0)
>> -                       return -1;
>> -               curclust = get_fatent(mydata, curclust);
>> -               if (CHECK_CLUST(curclust, mydata->fatsize)) {
>> -                       debug("curclust: 0x%x\n", curclust);
>> -                       printf("Invalid FAT entry\n");
>> -                       return -1;
>> -               }
>> -
>> -               if (get_cluster(mydata, curclust,
>> get_contents_vfatname_block,
>> -                               mydata->clust_size * mydata->sect_size) !=
>> 0) {
>> -                       debug("Error: reading directory block\n");
>> -                       return -1;
>> -               }
>> -
>> -               slotptr2 = (dir_slot *)get_contents_vfatname_block;
>> -               while (counter > 0) {
>> -                       if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
>> -                           & 0xff) != counter)
>> -                               return -1;
>> -                       slotptr2++;
>> -                       counter--;
>> -               }
>> -
>> -               /* Save the real directory entry */
>> -               realdent = (dir_entry *)slotptr2;
>> -               while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
>> -                       slotptr2--;
>> -                       slot2str(slotptr2, l_name, &idx);
>> -               }
>> -       } else {
>> -               /* Save the real directory entry */
>> -               realdent = (dir_entry *)slotptr;
>> -       }
>> -
>> -       do {
>> -               slotptr--;
>> -               if (slot2str(slotptr, l_name, &idx))
>> -                       break;
>> -       } while (!(slotptr->id & LAST_LONG_ENTRY_MASK));
>> -
>> -       l_name[idx] = '\0';
>> -       if (*l_name == DELETED_FLAG)
>> -               *l_name = '\0';
>> -       else if (*l_name == aRING)
>> -               *l_name = DELETED_FLAG;
>> -       downcase(l_name);
>> -
>> -       /* Return the real directory entry */
>> -       memcpy(retdent, realdent, sizeof(dir_entry));
>> -
>> -       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;
>>   }
>>   -/*
>> - * Get the directory entry associated with 'filename' from the directory
>> - * starting at 'startsect'
>> - */
>> +// These should probably DIAF..

Can you use /* ?

Perhaps a TODO here would help - are you suggesting using malloc()?

Did this patch go through patman/checkpatch?

Regards,
Simon

[U-Boot] [PATCH v2 4/8] fs: add fs_readdir()

[Simon Glass]

Hi Rob,

On 3 September 2017 at 23:16, Łukasz Majewski <lukma@denx.de> wrote:
> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>
>> Needed to support efi file protocol.  The fallback.efi loader wants
>> to be able to read the contents of the /EFI directory to find an OS
>> to boot.
>>
>> Modelled after POSIX opendir()/readdir()/closedir().  Unlike the other
>> fs APIs, this is stateful (ie. state is held in the FS_DIR "directory
>> stream"), to avoid re-traversing of the directory structure at each
>> step.  The directory stream must be released with closedir() when it
>> is no longer needed.
>>
>
> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>
>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>   disk/part.c    | 31 ++++++++++++--------
>>   fs/fs.c        | 91
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/fs.h   | 55 +++++++++++++++++++++++++++++++++++
>>   include/part.h |  4 +++
>>   4 files changed, 169 insertions(+), 12 deletions(-)
>>
>> diff --git a/disk/part.c b/disk/part.c
>> index c67fdacc79..aa9183d696 100644
>> --- a/disk/part.c
>> +++ b/disk/part.c
>> @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int
>> part,
>>         return -1;
>>   }
>>   +int part_get_info_whole_disk(struct blk_desc *dev_desc,
>> disk_partition_t *info)
>> +{
>> +       info->start = 0;
>> +       info->size = dev_desc->lba;
>> +       info->blksz = dev_desc->blksz;
>> +       info->bootable = 0;
>> +       strcpy((char *)info->type, BOOT_PART_TYPE);
>> +       strcpy((char *)info->name, "Whole Disk");
>> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)

Can you use if() instead of #if for this one? And below. It helps to
reduce the number of code paths at build-time.

>> +       info->uuid[0] = 0;
>> +#endif
>> +#ifdef CONFIG_PARTITION_TYPE_GUID

Here too. And below.

>> +       info->type_guid[0] = 0;
>> +#endif
>> +
>> +       return 0;
>> +}
>> +
>>   int blk_get_device_by_str(const char *ifname, const char
>> *dev_hwpart_str,
>>                           struct blk_desc **dev_desc)
>>   {
>> @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const
>> char *dev_part_str,
>>                 (*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);
>>   -             info->start = 0;
>> -               info->size = (*dev_desc)->lba;
>> -               info->blksz = (*dev_desc)->blksz;
>> -               info->bootable = 0;
>> -               strcpy((char *)info->type, BOOT_PART_TYPE);
>> -               strcpy((char *)info->name, "Whole Disk");
>> -#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
>> -               info->uuid[0] = 0;
>> -#endif
>> -#ifdef CONFIG_PARTITION_TYPE_GUID
>> -               info->type_guid[0] = 0;
>> -#endif
>> +               part_get_info_whole_disk(*dev_desc, info);
>>                 ret = 0;
>>                 goto cleanup;
>> diff --git a/fs/fs.c b/fs/fs.c
>> index 13cd3626c6..441c880654 100644
>> --- a/fs/fs.c
>> +++ b/fs/fs.c
>> @@ -21,6 +21,7 @@
>>   DECLARE_GLOBAL_DATA_PTR;
>>     static struct blk_desc *fs_dev_desc;
>> +static int fs_dev_part;
>>   static disk_partition_t fs_partition;
>>   static int fs_type = FS_TYPE_ANY;
>>   @@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)
>>         return -1;
>>   }
>>   +static inline int fs_opendir_unsupported(const char *filename, FS_DIR
>> **dirp)
>> +{
>> +       return -EACCES;
>> +}
>> +
>>   struct fstype_info {
>>         int fstype;
>>         char *name;
>> @@ -92,6 +98,9 @@ struct fstype_info {
>>                      loff_t len, loff_t *actwrite);
>>         void (*close)(void);
>>         int (*uuid)(char *uuid_str);
>> +       int (*opendir)(const char *filename, FS_DIR **dirp);
>> +       int (*readdir)(FS_DIR *dirp);
>> +       void (*closedir)(FS_DIR *dirp);

Please comment these. Also can you use struct instead of typedef?

>>   };
>>     static struct fstype_info fstypes[] = {
>> @@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {
>>                 .write = fs_write_unsupported,
>>   #endif
>>                 .uuid = fs_uuid_unsupported,
>> +               .opendir = fs_opendir_unsupported,
>>         },
>>   #endif
>>   #ifdef CONFIG_FS_EXT4
>> @@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {
>>                 .write = fs_write_unsupported,
>>   #endif
>>                 .uuid = ext4fs_uuid,
>> +               .opendir = fs_opendir_unsupported,
>>         },
>>   #endif
>>   #ifdef CONFIG_SANDBOX
>> @@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {
>>                 .read = fs_read_sandbox,
>>                 .write = fs_write_sandbox,
>>                 .uuid = fs_uuid_unsupported,
>> +               .opendir = fs_opendir_unsupported,
>>         },
>>   #endif
>>   #ifdef CONFIG_CMD_UBIFS
>> @@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {
>>                 .read = ubifs_read,
>>                 .write = fs_write_unsupported,
>>                 .uuid = fs_uuid_unsupported,
>> +               .opendir = fs_opendir_unsupported,
>>         },
>>   #endif
>>         {
>> @@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {
>>                 .read = fs_read_unsupported,
>>                 .write = fs_write_unsupported,
>>                 .uuid = fs_uuid_unsupported,
>> +               .opendir = fs_opendir_unsupported,
>>         },
>>   };
>>   @@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char
>> *dev_part_str, int fstype)
>>                 if (!info->probe(fs_dev_desc, &fs_partition)) {
>>                         fs_type = info->fstype;
>> +                       fs_dev_part = part;
>> +                       return 0;
>> +               }
>> +       }
>> +
>> +       return -1;
>> +}
>> +
>> +/* set current blk device w/ blk_desc + partition # */
>> +int fs_set_blk_dev2(struct blk_desc *desc, int part)

Please add a full function comment in the header file. See
fs_set_blk() which has a comment.

Also '2' is a bad name. How about a _with_part suffix, or something like that?

[...]

>> diff --git a/include/fs.h b/include/fs.h
>> index 2f2aca8378..0a6a366078 100644
>> --- a/include/fs.h
>> +++ b/include/fs.h
>> @@ -26,6 +26,8 @@
>>    */
>>   int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int
>> fstype);
>>   +int fs_set_blk_dev2(struct blk_desc *desc, int part);

Comment goes above this.

>> +
>>   /*
>>    * Print the list of files on the partition previously set by
>> fs_set_blk_dev(),
>>    * in directory "dirname".
>> @@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t
>> offset, loff_t len,
>>   int fs_write(const char *filename, ulong addr, loff_t offset, loff_t
>> len,
>>              loff_t *actwrite);
>>   +/* Add additional FS_DT_* as supported by additional filesystems:*/
>> +#define FS_DT_DIR  0x4       /* directory */
>> +#define FS_DT_REG  0x8       /* regular file */
>> +
>> +/*
>> + * A directory entry.
>> + */
>> +struct fs_dirent {
>> +       unsigned type;       /* one of FS_DT_* */

Is that a mask (so both can be set) or something else? If it is a mask
please say so.

>> +       loff_t size;

comment for this. Is it size of the file in bytes?

>> +       char name[256];
>> +};
>> +
>> +typedef struct _FS_DIR FS_DIR;
>> +

Please drop the typedef as it doesn't seem necessary

>> +/*
>> + * fs_opendir - Open a directory
>> + *
>> + * @filename: the path to directory to open
>> + * @return a pointer to the directory stream or NULL on error and errno
>> + *    set appropriately
>> + */
>> +FS_DIR *fs_opendir(const char *filename);
>> +
>> +/*
>> + * fs_readdir - Read the next directory entry in the directory stream.
>> + *
>> + * @dirp: the directory stream
>> + * @return the next directory entry (only valid until next fs_readdir()
>> or
>> + *    fs_closedir() call, do not attempt to free()) or NULL if the end of
>> + *    the directory is reached.
>> + */
>> +struct fs_dirent *fs_readdir(FS_DIR *dirp);
>> +
>> +/*
>> + * fs_closedir - close a directory stream
>> + *
>> + * @dirp: the directory stream
>> + */
>> +void fs_closedir(FS_DIR *dirp);
>> +
>> +/*
>> + * private to fs implementations, would be in fs.c but we need to let
>> + * implementations subclass:
>> + */
>> +
>> +struct _FS_DIR {

Lower case for struct names. Also please add struct member comments.

>> +       struct fs_dirent dirent;
>> +       /* private to fs layer: */
>> +       struct blk_desc *desc;
>> +       int part;
>> +};
>> +
>>   /*
>>    * Common implementation for various filesystem commands, optionally
>> limited
>>    * to a specific filesystem type via the fstype parameter.
>> diff --git a/include/part.h b/include/part.h
>> index 0cd803a933..48e8ff6d8a 100644
>> --- a/include/part.h
>> +++ b/include/part.h
>> @@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc
>> **blk_devp);
>>     /* disk/part.c */
>>   int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t
>> *info);
>> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t
>> *info);

Please fully comment new functions here.

>>   void part_print(struct blk_desc *dev_desc);
>>   void part_init(struct blk_desc *dev_desc);
>>   void dev_print(struct blk_desc *dev_desc);
>> @@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int
>> dev) { return NULL; }
>>     static inline int part_get_info(struct blk_desc *dev_desc, int part,
>>                                 disk_partition_t *info) { return -1; }
>> +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,
>> +                                          disk_partition_t *info)
>> +{ return -1; }
>>   static inline void part_print(struct blk_desc *dev_desc) {}
>>   static inline void part_init(struct blk_desc *dev_desc) {}
>>   static inline void dev_print(struct blk_desc *dev_desc) {}
>>

Regards,
Simon

[U-Boot] [PATCH v2 5/8] fs/fat: implement opendir/readdir/closedir

[Simon Glass]

On 3 September 2017 at 00:38, Rob Clark <robdclark@gmail.com> wrote:
> Implement the readdir interface using the directory iterators.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  fs/fat/fat.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 3193290434..d30ef3903b 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -14,6 +14,7 @@
>  #include <config.h>
>  #include <exports.h>
>  #include <fat.h>
> +#include <fs.h>
>  #include <asm/byteorder.h>
>  #include <part.h>
>  #include <malloc.h>
> @@ -1119,6 +1120,61 @@ int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
>         return ret;
>  }
>
> +typedef struct {
> +       FS_DIR parent;
> +       fsdata fsdata;
> +       fat_itr itr;
> +} fat_dir;
> +

Please drop the typedef.

This is a really nice implementation now.

[U-Boot] [PATCH v2 6/8] fat/fs: remove a bunch of dead code

[Simon Glass]

On 3 September 2017 at 00:38, Rob Clark <robdclark@gmail.com> wrote:
> Spotted by chance, when trying to remove file_fat_ls(), I noticed there
> were some dead users of the API.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> Acked-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
> ---
>  fs/fat/Makefile |   4 --
>  fs/fat/file.c   | 183 --------------------------------------------------------
>  include/fat.h   |  20 -------
>  3 files changed, 207 deletions(-)
>  delete mode 100644 fs/fat/file.c

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 7/8] fat/fs: move ls to generic implementation

[Simon Glass]

On 3 September 2017 at 00:38, Rob Clark <robdclark@gmail.com> wrote:
> Add a generic implementation of 'ls' using opendir/readdir/closedir, and
> replace fat's custom implementation.  Other filesystems should move to
> the generic implementation after they add opendir/readdir/closedir
> support.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  fs/fat/fat.c  | 32 --------------------------------
>  fs/fs.c       | 35 +++++++++++++++++++++++++++++++++--
>  include/fat.h |  5 ++++-
>  3 files changed, 37 insertions(+), 35 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

So if all filesystems implement this interface then we could move this
to function to the fs layer?

[U-Boot] [PATCH v2 8/8] fs/fat: fix case for FAT shortnames

[Simon Glass]

Hi Rob,

On 3 September 2017 at 23:22, Łukasz Majewski <lukma@denx.de> wrote:
> On 09/02/2017 06:38 PM, Rob Clark wrote:
>>
>> Noticed when comparing our output to linux.  There are some lcase bits
>> which control whether filename and/or extension should be downcase'd.
>
>
> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>
>
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>   fs/fat/fat.c       | 17 ++++++++++++-----
>>   fs/fat/fat_write.c |  4 ++--
>>   include/fat.h      |  3 +++
>>   3 files changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index fc3106aacb..ccbf7ba1c8 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -29,11 +29,13 @@ static const int vfat_enabled = 0;
>>   #endif
>>     /*
>> - * Convert a string to lowercase.
>> + * Convert a string to lowercase.  Converts at most 'len' characters,
>> + * 'len' may be larger than the length of 'str' if 'str' is NULL
>> + * terminated.
>>    */
>> -static void downcase(char *str)
>> +static void downcase(char *str, size_t len)
>>   {
>> -       while (*str != '\0') {
>> +       while (*str != '\0' && len--) {
>>                 *str = tolower(*str);
>>                 str++;
>>         }
>> @@ -131,10 +133,16 @@ static void get_name(dir_entry *dirent, char
>> *s_name)
>>         ptr = s_name;
>>         while (*ptr && *ptr != ' ')
>>                 ptr++;
>> +       if (dirent->lcase & CASE_LOWER_BASE)
>> +               downcase(s_name, (unsigned)(ptr - s_name));
>>         if (dirent->ext[0] && dirent->ext[0] != ' ') {
>> +               char *ext;
>> +
>>                 *ptr = '.';
>> -               ptr++;
>> +               ext = ++ptr;

I think this would be clearer as:

   *ptr++ = '.';
   ext = ptr;

But given that you only use ext once, can you just drop it?

>>                 memcpy(ptr, dirent->ext, 3);
>> +               if (dirent->lcase & CASE_LOWER_EXT)
>> +                       downcase(ext, 3);
>>                 ptr[3] = '\0';
>>                 while (*ptr && *ptr != ' ')
>>                         ptr++;
>> @@ -144,7 +152,6 @@ static void get_name(dir_entry *dirent, char *s_name)
>>                 *s_name = '\0';
>>         else if (*s_name == aRING)
>>                 *s_name = DELETED_FLAG;
>> -       downcase(s_name);
>>   }
>>     static int flush_dirty_fat_buffer(fsdata *mydata);
>> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
>> index 4ca024c208..655ad4ec84 100644
>> --- a/fs/fat/fat_write.c
>> +++ b/fs/fat/fat_write.c
>> @@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8
>> *cluster,
>>                 *l_name = '\0';
>>         else if (*l_name == aRING)
>>                 *l_name = DELETED_FLAG;
>> -       downcase(l_name);
>> +       downcase(l_name, ~0);

Could this be INT_MAX to indicate all? I think ~0 is a bit obscure

>>         /* Return the real directory entry */
>>         *retdent = realdent;
>> @@ -981,7 +981,7 @@ static int do_fat_write(const char *filename, void
>> *buffer, loff_t size,
>>         memcpy(l_filename, filename, name_len);
>>         l_filename[name_len] = 0; /* terminate the string */
>> -       downcase(l_filename);
>> +       downcase(l_filename, ~0);

here also

>>         startsect = mydata->rootdir_sect;
>>         retdent = find_directory_entry(mydata, startsect,
>> diff --git a/include/fat.h b/include/fat.h
>> index b2d4b952fd..5e4924316a 100644
>> --- a/include/fat.h
>> +++ b/include/fat.h
>> @@ -128,6 +128,9 @@ typedef struct volume_info
>>         /* Boot sign comes last, 2 bytes */
>>   } volume_info;
>>   +#define CASE_LOWER_BASE      8       /* base is lower case */
>> +#define CASE_LOWER_EXT 16      /* extension is lower case */

Where are these two used? If they are flags can you mention somewhere
what uses them?

>> +
>>   typedef struct dir_entry {
>>         char    name[8],ext[3]; /* Name and extension */
>>         __u8    attr;           /* Attribute bits */
>>
>

Regards,
Simon

[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Rob Clark]

On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:
>> Untangle directory traversal into a simple iterator, to replace the
>> existing multi-purpose do_fat_read_at() + get_dentfromdir().
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 326 insertions(+)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index e1c0a15dc7..c72d6ca931 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -1245,6 +1245,332 @@ exit:
>>         return ret;
>>  }
>>
>> +
>> +/*
>> + * Directory iterator, to simplify filesystem traversal
>> + */
>> +
>> +typedef struct {
>
> Please avoid using a typedef here. It seems like it could just be a struct.

I'm typically not a fan of 'typedef struct' but that seems to be the
style that fs/fat code was already using, and "when in Rome..."

> Also pleaee add a comment about what the struct is for
>
>> +       fsdata    *fsdata;
>> +       unsigned   cursect;
>> +       dir_entry *dent;
>> +       int        remaining;     /* remaining dent's in current cluster */
>> +       int        last_cluster;
>> +       int        is_root;
>> +
>> +       /* current iterator position values: */
>> +       char       l_name[VFAT_MAXLEN_BYTES];
>> +       char       s_name[14];
>> +       char      *name;          /* l_name if there is one, else s_name */
>> +
>> +       u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
>
> Some members are missing comments.
>
> Also I'm not too sure how the alignment works here. I don't see you
> creating one of these objects in this patch, but could you add a
> comment to the struct about how to create one? Do you use memalign()?

I was just creating them on the stack normally.. expecting the
compiler to dtrt because of the __aligned() on block[].

BR,
-R

[U-Boot] [PATCH v2 4/8] fs: add fs_readdir()

[Rob Clark]

On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 3 September 2017 at 23:16, Łukasz Majewski <lukma@denx.de> wrote:
>> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>>
>>> Needed to support efi file protocol.  The fallback.efi loader wants
>>> to be able to read the contents of the /EFI directory to find an OS
>>> to boot.
>>>
>>> Modelled after POSIX opendir()/readdir()/closedir().  Unlike the other
>>> fs APIs, this is stateful (ie. state is held in the FS_DIR "directory
>>> stream"), to avoid re-traversing of the directory structure at each
>>> step.  The directory stream must be released with closedir() when it
>>> is no longer needed.
>>>
>>
>> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>>
>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>   disk/part.c    | 31 ++++++++++++--------
>>>   fs/fs.c        | 91
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/fs.h   | 55 +++++++++++++++++++++++++++++++++++
>>>   include/part.h |  4 +++
>>>   4 files changed, 169 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/disk/part.c b/disk/part.c
>>> index c67fdacc79..aa9183d696 100644
>>> --- a/disk/part.c
>>> +++ b/disk/part.c
>>> @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int
>>> part,
>>>         return -1;
>>>   }
>>>   +int part_get_info_whole_disk(struct blk_desc *dev_desc,
>>> disk_partition_t *info)
>>> +{
>>> +       info->start = 0;
>>> +       info->size = dev_desc->lba;
>>> +       info->blksz = dev_desc->blksz;
>>> +       info->bootable = 0;
>>> +       strcpy((char *)info->type, BOOT_PART_TYPE);
>>> +       strcpy((char *)info->name, "Whole Disk");
>>> +#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
>
> Can you use if() instead of #if for this one? And below. It helps to
> reduce the number of code paths at build-time.

I don't think so, at least not without dropping the corresponding
#ifdef in disk_partition_t

>>> +       info->uuid[0] = 0;
>>> +#endif
>>> +#ifdef CONFIG_PARTITION_TYPE_GUID
>
> Here too. And below.

same thing

>>> +       info->type_guid[0] = 0;
>>> +#endif
>>> +
>>> +       return 0;
>>> +}
>>> +
>>>   int blk_get_device_by_str(const char *ifname, const char
>>> *dev_hwpart_str,
>>>                           struct blk_desc **dev_desc)
>>>   {
>>> @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const
>>> char *dev_part_str,
>>>                 (*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz);
>>>   -             info->start = 0;
>>> -               info->size = (*dev_desc)->lba;
>>> -               info->blksz = (*dev_desc)->blksz;
>>> -               info->bootable = 0;
>>> -               strcpy((char *)info->type, BOOT_PART_TYPE);
>>> -               strcpy((char *)info->name, "Whole Disk");
>>> -#if CONFIG_IS_ENABLED(PARTITION_UUIDS)
>>> -               info->uuid[0] = 0;
>>> -#endif
>>> -#ifdef CONFIG_PARTITION_TYPE_GUID
>>> -               info->type_guid[0] = 0;
>>> -#endif
>>> +               part_get_info_whole_disk(*dev_desc, info);
>>>                 ret = 0;
>>>                 goto cleanup;
>>> diff --git a/fs/fs.c b/fs/fs.c
>>> index 13cd3626c6..441c880654 100644
>>> --- a/fs/fs.c
>>> +++ b/fs/fs.c
>>> @@ -21,6 +21,7 @@
>>>   DECLARE_GLOBAL_DATA_PTR;
>>>     static struct blk_desc *fs_dev_desc;
>>> +static int fs_dev_part;
>>>   static disk_partition_t fs_partition;
>>>   static int fs_type = FS_TYPE_ANY;
>>>   @@ -69,6 +70,11 @@ static inline int fs_uuid_unsupported(char *uuid_str)
>>>         return -1;
>>>   }
>>>   +static inline int fs_opendir_unsupported(const char *filename, FS_DIR
>>> **dirp)
>>> +{
>>> +       return -EACCES;
>>> +}
>>> +
>>>   struct fstype_info {
>>>         int fstype;
>>>         char *name;
>>> @@ -92,6 +98,9 @@ struct fstype_info {
>>>                      loff_t len, loff_t *actwrite);
>>>         void (*close)(void);
>>>         int (*uuid)(char *uuid_str);
>>> +       int (*opendir)(const char *filename, FS_DIR **dirp);
>>> +       int (*readdir)(FS_DIR *dirp);
>>> +       void (*closedir)(FS_DIR *dirp);
>
> Please comment these. Also can you use struct instead of typedef?

typedef-struct-caps here was based on how posix readdir() works.  I
guess we can deviate, it isn't 100% clone of readdir().. but I figured
it was more clear to be more similar to posix.

>>>   };
>>>     static struct fstype_info fstypes[] = {
>>> @@ -112,6 +121,7 @@ static struct fstype_info fstypes[] = {
>>>                 .write = fs_write_unsupported,
>>>   #endif
>>>                 .uuid = fs_uuid_unsupported,
>>> +               .opendir = fs_opendir_unsupported,
>>>         },
>>>   #endif
>>>   #ifdef CONFIG_FS_EXT4
>>> @@ -131,6 +141,7 @@ static struct fstype_info fstypes[] = {
>>>                 .write = fs_write_unsupported,
>>>   #endif
>>>                 .uuid = ext4fs_uuid,
>>> +               .opendir = fs_opendir_unsupported,
>>>         },
>>>   #endif
>>>   #ifdef CONFIG_SANDBOX
>>> @@ -146,6 +157,7 @@ static struct fstype_info fstypes[] = {
>>>                 .read = fs_read_sandbox,
>>>                 .write = fs_write_sandbox,
>>>                 .uuid = fs_uuid_unsupported,
>>> +               .opendir = fs_opendir_unsupported,
>>>         },
>>>   #endif
>>>   #ifdef CONFIG_CMD_UBIFS
>>> @@ -161,6 +173,7 @@ static struct fstype_info fstypes[] = {
>>>                 .read = ubifs_read,
>>>                 .write = fs_write_unsupported,
>>>                 .uuid = fs_uuid_unsupported,
>>> +               .opendir = fs_opendir_unsupported,
>>>         },
>>>   #endif
>>>         {
>>> @@ -175,6 +188,7 @@ static struct fstype_info fstypes[] = {
>>>                 .read = fs_read_unsupported,
>>>                 .write = fs_write_unsupported,
>>>                 .uuid = fs_uuid_unsupported,
>>> +               .opendir = fs_opendir_unsupported,
>>>         },
>>>   };
>>>   @@ -228,6 +242,31 @@ int fs_set_blk_dev(const char *ifname, const char
>>> *dev_part_str, int fstype)
>>>                 if (!info->probe(fs_dev_desc, &fs_partition)) {
>>>                         fs_type = info->fstype;
>>> +                       fs_dev_part = part;
>>> +                       return 0;
>>> +               }
>>> +       }
>>> +
>>> +       return -1;
>>> +}
>>> +
>>> +/* set current blk device w/ blk_desc + partition # */
>>> +int fs_set_blk_dev2(struct blk_desc *desc, int part)
>
> Please add a full function comment in the header file. See
> fs_set_blk() which has a comment.
>
> Also '2' is a bad name. How about a _with_part suffix, or something like that?

yeah, "2" was me running out of creativity.. _with_part sounds reasonable.

> [...]
>
>>> diff --git a/include/fs.h b/include/fs.h
>>> index 2f2aca8378..0a6a366078 100644
>>> --- a/include/fs.h
>>> +++ b/include/fs.h
>>> @@ -26,6 +26,8 @@
>>>    */
>>>   int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int
>>> fstype);
>>>   +int fs_set_blk_dev2(struct blk_desc *desc, int part);
>
> Comment goes above this.
>
>>> +
>>>   /*
>>>    * Print the list of files on the partition previously set by
>>> fs_set_blk_dev(),
>>>    * in directory "dirname".
>>> @@ -78,6 +80,59 @@ int fs_read(const char *filename, ulong addr, loff_t
>>> offset, loff_t len,
>>>   int fs_write(const char *filename, ulong addr, loff_t offset, loff_t
>>> len,
>>>              loff_t *actwrite);
>>>   +/* Add additional FS_DT_* as supported by additional filesystems:*/
>>> +#define FS_DT_DIR  0x4       /* directory */
>>> +#define FS_DT_REG  0x8       /* regular file */
>>> +
>>> +/*
>>> + * A directory entry.
>>> + */
>>> +struct fs_dirent {
>>> +       unsigned type;       /* one of FS_DT_* */
>
> Is that a mask (so both can be set) or something else? If it is a mask
> please say so.

It is not a mask, despite how the corresponding readdir() DT_* are
defined.  They match DT_* constants, or rather the subset of them that
make sense (ie. we don't have pipes or sockets.. maybe at some point
when someone implements this for ext4 it would be worth adding
FS_DT_LNK for symlinks)

BR,
-R


>>> +       loff_t size;
>
> comment for this. Is it size of the file in bytes?
>
>>> +       char name[256];
>>> +};
>>> +
>>> +typedef struct _FS_DIR FS_DIR;
>>> +
>
> Please drop the typedef as it doesn't seem necessary
>
>>> +/*
>>> + * fs_opendir - Open a directory
>>> + *
>>> + * @filename: the path to directory to open
>>> + * @return a pointer to the directory stream or NULL on error and errno
>>> + *    set appropriately
>>> + */
>>> +FS_DIR *fs_opendir(const char *filename);
>>> +
>>> +/*
>>> + * fs_readdir - Read the next directory entry in the directory stream.
>>> + *
>>> + * @dirp: the directory stream
>>> + * @return the next directory entry (only valid until next fs_readdir()
>>> or
>>> + *    fs_closedir() call, do not attempt to free()) or NULL if the end of
>>> + *    the directory is reached.
>>> + */
>>> +struct fs_dirent *fs_readdir(FS_DIR *dirp);
>>> +
>>> +/*
>>> + * fs_closedir - close a directory stream
>>> + *
>>> + * @dirp: the directory stream
>>> + */
>>> +void fs_closedir(FS_DIR *dirp);
>>> +
>>> +/*
>>> + * private to fs implementations, would be in fs.c but we need to let
>>> + * implementations subclass:
>>> + */
>>> +
>>> +struct _FS_DIR {
>
> Lower case for struct names. Also please add struct member comments.
>
>>> +       struct fs_dirent dirent;
>>> +       /* private to fs layer: */
>>> +       struct blk_desc *desc;
>>> +       int part;
>>> +};
>>> +
>>>   /*
>>>    * Common implementation for various filesystem commands, optionally
>>> limited
>>>    * to a specific filesystem type via the fstype parameter.
>>> diff --git a/include/part.h b/include/part.h
>>> index 0cd803a933..48e8ff6d8a 100644
>>> --- a/include/part.h
>>> +++ b/include/part.h
>>> @@ -98,6 +98,7 @@ int host_get_dev_err(int dev, struct blk_desc
>>> **blk_devp);
>>>     /* disk/part.c */
>>>   int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t
>>> *info);
>>> +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t
>>> *info);
>
> Please fully comment new functions here.
>
>>>   void part_print(struct blk_desc *dev_desc);
>>>   void part_init(struct blk_desc *dev_desc);
>>>   void dev_print(struct blk_desc *dev_desc);
>>> @@ -203,6 +204,9 @@ static inline struct blk_desc *mg_disk_get_dev(int
>>> dev) { return NULL; }
>>>     static inline int part_get_info(struct blk_desc *dev_desc, int part,
>>>                                 disk_partition_t *info) { return -1; }
>>> +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc,
>>> +                                          disk_partition_t *info)
>>> +{ return -1; }
>>>   static inline void part_print(struct blk_desc *dev_desc) {}
>>>   static inline void part_init(struct blk_desc *dev_desc) {}
>>>   static inline void dev_print(struct blk_desc *dev_desc) {}
>>>
>
> Regards,
> Simon

[U-Boot] [PATCH v2 7/8] fat/fs: move ls to generic implementation

[Rob Clark]

On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
> On 3 September 2017 at 00:38, Rob Clark <robdclark@gmail.com> wrote:
>> Add a generic implementation of 'ls' using opendir/readdir/closedir, and
>> replace fat's custom implementation.  Other filesystems should move to
>> the generic implementation after they add opendir/readdir/closedir
>> support.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  fs/fat/fat.c  | 32 --------------------------------
>>  fs/fs.c       | 35 +++++++++++++++++++++++++++++++++--
>>  include/fat.h |  5 ++++-
>>  3 files changed, 37 insertions(+), 35 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> So if all filesystems implement this interface then we could move this
> to function to the fs layer?

yes.. there are some slight differences in output for other fs's but I
guess since u-boot doesn't make that easy for a script to capture we
can probably ignore it..

BR,
-R

[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Rob Clark]

On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 3 September 2017 at 23:08, Łukasz Majewski <lukma@denx.de> wrote:
>> On 09/02/2017 06:37 PM, Rob Clark wrote:
>>>
>>> And drop a whole lot of ugly code!
>
> :-)
>
>>
>>
>> +1
>>
>> Reviewed-by: Łukasz Majewski <lukma@denx.de>
>>
>>
>>
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>   fs/fat/fat.c  | 723
>>> ++++++----------------------------------------------------
>>>   include/fat.h |   6 -
>>>   2 files changed, 75 insertions(+), 654 deletions(-)
>>>
>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>> index c72d6ca931..3193290434 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)
>>>   }
>>>     /*
>>> - * Get the first occurence of a directory delimiter ('/' or '\') in a
>>> string.
>>> - * Return index into string if found, -1 otherwise.
>>> - */
>>> -static int dirdelim(char *str)
>>> -{
>>> -       char *start = str;
>>> -
>>> -       while (*str != '\0') {
>>> -               if (ISDIRDELIM(*str))
>>> -                       return str - start;
>>> -               str++;
>>> -       }
>>> -       return -1;
>>> -}
>>> -
>>> -/*
>>>    * 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;
>>>   }
>>>   -/*
>>> - * Extract the full long filename starting at 'retdent' (which is really
>>> - * a slot) into 'l_name'. If successful also copy the real directory
>>> entry
>>> - * into 'retdent'
>>> - * Return 0 on success, -1 otherwise.
>>> - */
>>> -static int
>>> -get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
>>> -            dir_entry *retdent, char *l_name)
>>> -{
>>> -       dir_entry *realdent;
>>> -       dir_slot *slotptr = (dir_slot *)retdent;
>>> -       __u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ?
>>> -                                                       PREFETCH_BLOCKS :
>>> -
>>> mydata->clust_size);
>>> -       __u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
>>> -       int idx = 0;
>>> -
>>> -       if (counter > VFAT_MAXSEQ) {
>>> -               debug("Error: VFAT name is too long\n");
>>> -               return -1;
>>> -       }
>>> -
>>> -       while ((__u8 *)slotptr < buflimit) {
>>> -               if (counter == 0)
>>> -                       break;
>>> -               if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) !=
>>> counter)
>>> -                       return -1;
>>> -               slotptr++;
>>> -               counter--;
>>> -       }
>>> -
>>> -       if ((__u8 *)slotptr >= buflimit) {
>>> -               dir_slot *slotptr2;
>>> -
>>> -               if (curclust == 0)
>>> -                       return -1;
>>> -               curclust = get_fatent(mydata, curclust);
>>> -               if (CHECK_CLUST(curclust, mydata->fatsize)) {
>>> -                       debug("curclust: 0x%x\n", curclust);
>>> -                       printf("Invalid FAT entry\n");
>>> -                       return -1;
>>> -               }
>>> -
>>> -               if (get_cluster(mydata, curclust,
>>> get_contents_vfatname_block,
>>> -                               mydata->clust_size * mydata->sect_size) !=
>>> 0) {
>>> -                       debug("Error: reading directory block\n");
>>> -                       return -1;
>>> -               }
>>> -
>>> -               slotptr2 = (dir_slot *)get_contents_vfatname_block;
>>> -               while (counter > 0) {
>>> -                       if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
>>> -                           & 0xff) != counter)
>>> -                               return -1;
>>> -                       slotptr2++;
>>> -                       counter--;
>>> -               }
>>> -
>>> -               /* Save the real directory entry */
>>> -               realdent = (dir_entry *)slotptr2;
>>> -               while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
>>> -                       slotptr2--;
>>> -                       slot2str(slotptr2, l_name, &idx);
>>> -               }
>>> -       } else {
>>> -               /* Save the real directory entry */
>>> -               realdent = (dir_entry *)slotptr;
>>> -       }
>>> -
>>> -       do {
>>> -               slotptr--;
>>> -               if (slot2str(slotptr, l_name, &idx))
>>> -                       break;
>>> -       } while (!(slotptr->id & LAST_LONG_ENTRY_MASK));
>>> -
>>> -       l_name[idx] = '\0';
>>> -       if (*l_name == DELETED_FLAG)
>>> -               *l_name = '\0';
>>> -       else if (*l_name == aRING)
>>> -               *l_name = DELETED_FLAG;
>>> -       downcase(l_name);
>>> -
>>> -       /* Return the real directory entry */
>>> -       memcpy(retdent, realdent, sizeof(dir_entry));
>>> -
>>> -       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;
>>>   }
>>>   -/*
>>> - * Get the directory entry associated with 'filename' from the directory
>>> - * starting at 'startsect'
>>> - */
>>> +// These should probably DIAF..
>
> Can you use /* ?
>
> Perhaps a TODO here would help - are you suggesting using malloc()?

I'll convert to /* TODO .. */

I didn't mean to use malloc, but iirc at least some of this goes away
when fat_write.c gets converted to use directory iterators.. I think
these are only used directly or indirectly by do_fat_write()..

I may have to tackle fat_write.c in the near future for UEFI SCT test
suite (un)fortunately..  might be after next merge-window but I think
that TODO can be tackled in the near future.

BR,
-R


> Did this patch go through patman/checkpatch?
>
> Regards,
> Simon

[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Simon Glass]

Hi Rob,

On 5 September 2017 at 03:54, Rob Clark <robdclark@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Rob,
>>
>> On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:
>>> Untangle directory traversal into a simple iterator, to replace the
>>> existing multi-purpose do_fat_read_at() + get_dentfromdir().
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 326 insertions(+)
>>>
>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>> index e1c0a15dc7..c72d6ca931 100644
>>> --- a/fs/fat/fat.c
>>> +++ b/fs/fat/fat.c
>>> @@ -1245,6 +1245,332 @@ exit:
>>>         return ret;
>>>  }
>>>
>>> +
>>> +/*
>>> + * Directory iterator, to simplify filesystem traversal
>>> + */
>>> +
>>> +typedef struct {
>>
>> Please avoid using a typedef here. It seems like it could just be a struct.
>
> I'm typically not a fan of 'typedef struct' but that seems to be the
> style that fs/fat code was already using, and "when in Rome..."

Sure, but let's move to Venice as it is less dusty.

>
>> Also pleaee add a comment about what the struct is for
>>
>>> +       fsdata    *fsdata;
>>> +       unsigned   cursect;
>>> +       dir_entry *dent;
>>> +       int        remaining;     /* remaining dent's in current cluster */
>>> +       int        last_cluster;
>>> +       int        is_root;
>>> +
>>> +       /* current iterator position values: */
>>> +       char       l_name[VFAT_MAXLEN_BYTES];
>>> +       char       s_name[14];
>>> +       char      *name;          /* l_name if there is one, else s_name */
>>> +
>>> +       u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
>>
>> Some members are missing comments.
>>
>> Also I'm not too sure how the alignment works here. I don't see you
>> creating one of these objects in this patch, but could you add a
>> comment to the struct about how to create one? Do you use memalign()?
>
> I was just creating them on the stack normally.. expecting the
> compiler to dtrt because of the __aligned() on block[].

Well I think that works. OK.

Regards,
Simon

[U-Boot] [PATCH v2 2/8] fs/fat: introduce new director iterators

[Rob Clark]

On Sat, Sep 9, 2017 at 12:55 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 5 September 2017 at 03:54, Rob Clark <robdclark@gmail.com> wrote:
>> On Tue, Sep 5, 2017 at 4:56 AM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi Rob,
>>>
>>> On 3 September 2017 at 00:37, Rob Clark <robdclark@gmail.com> wrote:
>>>> Untangle directory traversal into a simple iterator, to replace the
>>>> existing multi-purpose do_fat_read_at() + get_dentfromdir().
>>>>
>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>> ---
>>>>  fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 326 insertions(+)
>>>>
>>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>>> index e1c0a15dc7..c72d6ca931 100644
>>>> --- a/fs/fat/fat.c
>>>> +++ b/fs/fat/fat.c
>>>> @@ -1245,6 +1245,332 @@ exit:
>>>>         return ret;
>>>>  }
>>>>
>>>> +
>>>> +/*
>>>> + * Directory iterator, to simplify filesystem traversal
>>>> + */
>>>> +
>>>> +typedef struct {
>>>
>>> Please avoid using a typedef here. It seems like it could just be a struct.
>>
>> I'm typically not a fan of 'typedef struct' but that seems to be the
>> style that fs/fat code was already using, and "when in Rome..."
>
> Sure, but let's move to Venice as it is less dusty.
>

I'm not a huge fan of mixing styles.. but what I'd propose instead is this:

It turns out fat_write.c needs a lot of work for SCT too (ability to
write files in subdirectories, and write files w/ !=0 offset), so that
we can actually see the test results.. and before converting write
paths to use the new directory iterator, I was considering
re-organizing the code a bit, ie. move directory traversal code to
fs/fat/dir.c or util.c, move most of include/fat.h into a private
header, dropping the #include "fat.c" hack, etc.  That would be a good
time for flag-day cleanups like untypedef'ifying and perhaps cleaning
up all the #define macros that implicitly depend on having a 'mydata'
var.

So I'd suggest to leave it as-is for now, and change things to 'struct
foo' as part of the re-org.

That leaves FS_DIR in the later readdir patch.. I could change that to
'struct fs_dir_stream'.  That makes it not match the posix API it is
modelled after as closely, but maybe that is ok.

BR,
-R

[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Rob Clark]

On Mon, Aug 14, 2017 at 10:39 AM, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Aug 14, 2017 at 9:47 AM, Brüns, Stefan
> <Stefan.Bruens@rwth-aachen.de> wrote:
>
>> 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..
>

ok, looks like what I needed was:

@@ -696,7 +696,7 @@ static void fat_itr_child(fat_itr *itr, fat_itr *parent)
         itr->cursect = itr->fsdata->data_begin +
             (clustnum * itr->fsdata->clust_size);
     } else {
-        itr->cursect = 0;
+        itr->cursect = parent->fsdata->rootdir_sect;
     }
     itr->dent = NULL;
     itr->remaining = 0;


fixed up locally

BR,
-R

[U-Boot] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Rob Clark]

On Mon, Aug 14, 2017 at 9:47 AM, Brüns, Stefan
<Stefan.Bruens@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 <robdclark@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] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Brüns, Stefan]

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 <robdclark@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] [PATCH v2 3/8] fat/fs: convert to directory iterators

[Rob Clark]

And drop a whole lot of ugly code!

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 fs/fat/fat.c  | 723 ++++++----------------------------------------------------
 include/fat.h |   6 -
 2 files changed, 75 insertions(+), 654 deletions(-)

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)
 }
 
 /*
- * Get the first occurence of a directory delimiter ('/' or '\') in a string.
- * Return index into string if found, -1 otherwise.
- */
-static int dirdelim(char *str)
-{
-	char *start = str;
-
-	while (*str != '\0') {
-		if (ISDIRDELIM(*str))
-			return str - start;
-		str++;
-	}
-	return -1;
-}
-
-/*
  * 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;
 }
 
-/*
- * Extract the full long filename starting at 'retdent' (which is really
- * a slot) into 'l_name'. If successful also copy the real directory entry
- * into 'retdent'
- * Return 0 on success, -1 otherwise.
- */
-static int
-get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
-	     dir_entry *retdent, char *l_name)
-{
-	dir_entry *realdent;
-	dir_slot *slotptr = (dir_slot *)retdent;
-	__u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ?
-							PREFETCH_BLOCKS :
-							mydata->clust_size);
-	__u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff;
-	int idx = 0;
-
-	if (counter > VFAT_MAXSEQ) {
-		debug("Error: VFAT name is too long\n");
-		return -1;
-	}
-
-	while ((__u8 *)slotptr < buflimit) {
-		if (counter == 0)
-			break;
-		if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) != counter)
-			return -1;
-		slotptr++;
-		counter--;
-	}
-
-	if ((__u8 *)slotptr >= buflimit) {
-		dir_slot *slotptr2;
-
-		if (curclust == 0)
-			return -1;
-		curclust = get_fatent(mydata, curclust);
-		if (CHECK_CLUST(curclust, mydata->fatsize)) {
-			debug("curclust: 0x%x\n", curclust);
-			printf("Invalid FAT entry\n");
-			return -1;
-		}
-
-		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
-				mydata->clust_size * mydata->sect_size) != 0) {
-			debug("Error: reading directory block\n");
-			return -1;
-		}
-
-		slotptr2 = (dir_slot *)get_contents_vfatname_block;
-		while (counter > 0) {
-			if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
-			    & 0xff) != counter)
-				return -1;
-			slotptr2++;
-			counter--;
-		}
-
-		/* Save the real directory entry */
-		realdent = (dir_entry *)slotptr2;
-		while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
-			slotptr2--;
-			slot2str(slotptr2, l_name, &idx);
-		}
-	} else {
-		/* Save the real directory entry */
-		realdent = (dir_entry *)slotptr;
-	}
-
-	do {
-		slotptr--;
-		if (slot2str(slotptr, l_name, &idx))
-			break;
-	} while (!(slotptr->id & LAST_LONG_ENTRY_MASK));
-
-	l_name[idx] = '\0';
-	if (*l_name == DELETED_FLAG)
-		*l_name = '\0';
-	else if (*l_name == aRING)
-		*l_name = DELETED_FLAG;
-	downcase(l_name);
-
-	/* Return the real directory entry */
-	memcpy(retdent, realdent, sizeof(dir_entry));
-
-	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;
 }
 
-/*
- * Get the directory entry associated with 'filename' from the directory
- * starting at 'startsect'
- */
+// These should probably DIAF..
 __u8 get_dentfromdir_block[MAX_CLUSTSIZE]
 	__aligned(ARCH_DMA_MINALIGN);
-
-static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,
-				  char *filename, dir_entry *retdent,
-				  int dols)
-{
-	__u16 prevcksum = 0xffff;
-	__u32 curclust = START(retdent);
-	int files = 0, dirs = 0;
-
-	debug("get_dentfromdir: %s\n", filename);
-
-	while (1) {
-		dir_entry *dentptr;
-
-		int i;
-
-		if (get_cluster(mydata, curclust, get_dentfromdir_block,
-				mydata->clust_size * mydata->sect_size) != 0) {
-			debug("Error: reading directory block\n");
-			return NULL;
-		}
-
-		dentptr = (dir_entry *)get_dentfromdir_block;
-
-		for (i = 0; i < DIRENTSPERCLUST; i++) {
-			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
-
-			l_name[0] = '\0';
-			if (dentptr->name[0] == DELETED_FLAG) {
-				dentptr++;
-				continue;
-			}
-			if ((dentptr->attr & ATTR_VOLUME)) {
-				if (vfat_enabled &&
-				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
-				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
-					prevcksum = ((dir_slot *)dentptr)->alias_checksum;
-					get_vfatname(mydata, curclust,
-						     get_dentfromdir_block,
-						     dentptr, l_name);
-					if (dols) {
-						int isdir;
-						char dirc;
-						int doit = 0;
-
-						isdir = (dentptr->attr & ATTR_DIR);
-
-						if (isdir) {
-							dirs++;
-							dirc = '/';
-							doit = 1;
-						} else {
-							dirc = ' ';
-							if (l_name[0] != 0) {
-								files++;
-								doit = 1;
-							}
-						}
-						if (doit) {
-							if (dirc == ' ') {
-								printf(" %8u   %s%c\n",
-								       FAT2CPU32(dentptr->size),
-									l_name,
-									dirc);
-							} else {
-								printf("            %s%c\n",
-									l_name,
-									dirc);
-							}
-						}
-						dentptr++;
-						continue;
-					}
-					debug("vfatname: |%s|\n", l_name);
-				} else {
-					/* Volume label or VFAT entry */
-					dentptr++;
-					continue;
-				}
-			}
-			if (dentptr->name[0] == 0) {
-				if (dols) {
-					printf("\n%d file(s), %d dir(s)\n\n",
-						files, dirs);
-				}
-				debug("Dentname == NULL - %d\n", i);
-				return NULL;
-			}
-			if (vfat_enabled) {
-				__u8 csum = mkcksum(dentptr->name, dentptr->ext);
-				if (dols && csum == prevcksum) {
-					prevcksum = 0xffff;
-					dentptr++;
-					continue;
-				}
-			}
-
-			get_name(dentptr, s_name);
-			if (dols) {
-				int isdir = (dentptr->attr & ATTR_DIR);
-				char dirc;
-				int doit = 0;
-
-				if (isdir) {
-					dirs++;
-					dirc = '/';
-					doit = 1;
-				} else {
-					dirc = ' ';
-					if (s_name[0] != 0) {
-						files++;
-						doit = 1;
-					}
-				}
-
-				if (doit) {
-					if (dirc == ' ') {
-						printf(" %8u   %s%c\n",
-						       FAT2CPU32(dentptr->size),
-							s_name, dirc);
-					} else {
-						printf("            %s%c\n",
-							s_name, dirc);
-					}
-				}
-
-				dentptr++;
-				continue;
-			}
-
-			if (strcmp(filename, s_name)
-			    && strcmp(filename, l_name)) {
-				debug("Mismatch: |%s|%s|\n", s_name, l_name);
-				dentptr++;
-				continue;
-			}
-
-			memcpy(retdent, dentptr, sizeof(dir_entry));
-
-			debug("DentName: %s", s_name);
-			debug(", start: 0x%x", START(dentptr));
-			debug(", size:  0x%x %s\n",
-			      FAT2CPU32(dentptr->size),
-			      (dentptr->attr & ATTR_DIR) ? "(DIR)" : "");
-
-			return retdent;
-		}
-
-		curclust = get_fatent(mydata, curclust);
-		if (CHECK_CLUST(curclust, mydata->fatsize)) {
-			debug("curclust: 0x%x\n", curclust);
-			printf("Invalid FAT entry\n");
-			return NULL;
-		}
-	}
-
-	return NULL;
-}
+__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
+	__aligned(ARCH_DMA_MINALIGN);
 
 /*
  * Read boot sector and volume info from a FAT filesystem
@@ -877,374 +613,6 @@ static int get_fs_info(fsdata *mydata)
 	return 0;
 }
 
-__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
-	__aligned(ARCH_DMA_MINALIGN);
-
-int do_fat_read_at(const char *filename, loff_t pos, void *buffer,
-		   loff_t maxsize, int dols, int dogetsize, loff_t *size)
-{
-	char fnamecopy[2048];
-	fsdata datablock;
-	fsdata *mydata = &datablock;
-	dir_entry *dentptr = NULL;
-	__u16 prevcksum = 0xffff;
-	char *subname = "";
-	__u32 cursect;
-	int idx, isdir = 0;
-	int files = 0, dirs = 0;
-	int ret = -1;
-	int firsttime;
-	__u32 root_cluster = 0;
-	__u32 read_blk;
-	int rootdir_size = 0;
-	int buffer_blk_cnt;
-	int do_read;
-	__u8 *dir_ptr;
-
-	if (get_fs_info(mydata))
-		return -1;
-
-	cursect = mydata->rootdir_sect;
-
-	/* "cwd" is always the root... */
-	while (ISDIRDELIM(*filename))
-		filename++;
-
-	/* Make a copy of the filename and convert it to lowercase */
-	strcpy(fnamecopy, filename);
-	downcase(fnamecopy);
-
-root_reparse:
-	if (*fnamecopy == '\0') {
-		if (!dols)
-			goto exit;
-
-		dols = LS_ROOT;
-	} else if ((idx = dirdelim(fnamecopy)) >= 0) {
-		isdir = 1;
-		fnamecopy[idx] = '\0';
-		subname = fnamecopy + idx + 1;
-
-		/* Handle multiple delimiters */
-		while (ISDIRDELIM(*subname))
-			subname++;
-	} else if (dols) {
-		isdir = 1;
-	}
-
-	buffer_blk_cnt = 0;
-	firsttime = 1;
-	while (1) {
-		int i;
-
-		if (mydata->fatsize == 32 || firsttime) {
-			dir_ptr = do_fat_read_at_block;
-			firsttime = 0;
-		} else {
-			/**
-			 * FAT16 sector buffer modification:
-			 * Each loop, the second buffered block is moved to
-			 * the buffer begin, and two next sectors are read
-			 * next to the previously moved one. So the sector
-			 * buffer keeps always 3 sectors for fat16.
-			 * And the current sector is the buffer second sector
-			 * beside the "firsttime" read, when it is the first one.
-			 *
-			 * PREFETCH_BLOCKS is 2 for FAT16 == loop[0:1]
-			 * n = computed root dir sector
-			 * loop |  cursect-1  | cursect    | cursect+1  |
-			 *   0  |  sector n+0 | sector n+1 | none       |
-			 *   1  |  none       | sector n+0 | sector n+1 |
-			 *   0  |  sector n+1 | sector n+2 | sector n+3 |
-			 *   1  |  sector n+3 | ...
-			*/
-			dir_ptr = (do_fat_read_at_block + mydata->sect_size);
-			memcpy(do_fat_read_at_block, dir_ptr, mydata->sect_size);
-		}
-
-		do_read = 1;
-
-		if (mydata->fatsize == 32 && buffer_blk_cnt)
-			do_read = 0;
-
-		if (do_read) {
-			read_blk = (mydata->fatsize == 32) ?
-				    mydata->clust_size : PREFETCH_BLOCKS;
-
-			debug("FAT read(sect=%d, cnt:%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n",
-				cursect, read_blk, mydata->clust_size, DIRENTSPERBLOCK);
-
-			if (disk_read(cursect, read_blk, dir_ptr) < 0) {
-				debug("Error: reading rootdir block\n");
-				goto exit;
-			}
-
-			dentptr = (dir_entry *)dir_ptr;
-		}
-
-		for (i = 0; i < DIRENTSPERBLOCK; i++) {
-			char s_name[14], l_name[VFAT_MAXLEN_BYTES];
-			__u8 csum;
-
-			l_name[0] = '\0';
-			if (dentptr->name[0] == DELETED_FLAG) {
-				dentptr++;
-				continue;
-			}
-
-			if (vfat_enabled)
-				csum = mkcksum(dentptr->name, dentptr->ext);
-
-			if (dentptr->attr & ATTR_VOLUME) {
-				if (vfat_enabled &&
-				    (dentptr->attr & ATTR_VFAT) == ATTR_VFAT &&
-				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
-					prevcksum =
-						((dir_slot *)dentptr)->alias_checksum;
-
-					get_vfatname(mydata,
-						     root_cluster,
-						     dir_ptr,
-						     dentptr, l_name);
-
-					if (dols == LS_ROOT) {
-						char dirc;
-						int doit = 0;
-						int isdir =
-							(dentptr->attr & ATTR_DIR);
-
-						if (isdir) {
-							dirs++;
-							dirc = '/';
-							doit = 1;
-						} else {
-							dirc = ' ';
-							if (l_name[0] != 0) {
-								files++;
-								doit = 1;
-							}
-						}
-						if (doit) {
-							if (dirc == ' ') {
-								printf(" %8u   %s%c\n",
-								       FAT2CPU32(dentptr->size),
-									l_name,
-									dirc);
-							} else {
-								printf("            %s%c\n",
-									l_name,
-									dirc);
-							}
-						}
-						dentptr++;
-						continue;
-					}
-					debug("Rootvfatname: |%s|\n",
-					       l_name);
-				} else {
-					/* Volume label or VFAT entry */
-					dentptr++;
-					continue;
-				}
-			} else if (dentptr->name[0] == 0) {
-				debug("RootDentname == NULL - %d\n", i);
-				if (dols == LS_ROOT) {
-					printf("\n%d file(s), %d dir(s)\n\n",
-						files, dirs);
-					ret = 0;
-				}
-				goto exit;
-			}
-			else if (vfat_enabled &&
-				 dols == LS_ROOT && csum == prevcksum) {
-				prevcksum = 0xffff;
-				dentptr++;
-				continue;
-			}
-
-			get_name(dentptr, s_name);
-
-			if (dols == LS_ROOT) {
-				int isdir = (dentptr->attr & ATTR_DIR);
-				char dirc;
-				int doit = 0;
-
-				if (isdir) {
-					dirc = '/';
-					if (s_name[0] != 0) {
-						dirs++;
-						doit = 1;
-					}
-				} else {
-					dirc = ' ';
-					if (s_name[0] != 0) {
-						files++;
-						doit = 1;
-					}
-				}
-				if (doit) {
-					if (dirc == ' ') {
-						printf(" %8u   %s%c\n",
-						       FAT2CPU32(dentptr->size),
-							s_name, dirc);
-					} else {
-						printf("            %s%c\n",
-							s_name, dirc);
-					}
-				}
-				dentptr++;
-				continue;
-			}
-
-			if (strcmp(fnamecopy, s_name)
-			    && strcmp(fnamecopy, l_name)) {
-				debug("RootMismatch: |%s|%s|\n", s_name,
-				       l_name);
-				dentptr++;
-				continue;
-			}
-
-			if (isdir && !(dentptr->attr & ATTR_DIR))
-				goto exit;
-
-			debug("RootName: %s", s_name);
-			debug(", start: 0x%x", START(dentptr));
-			debug(", size:  0x%x %s\n",
-			       FAT2CPU32(dentptr->size),
-			       isdir ? "(DIR)" : "");
-
-			goto rootdir_done;	/* We got a match */
-		}
-		debug("END LOOP: buffer_blk_cnt=%d   clust_size=%d\n", buffer_blk_cnt,
-		       mydata->clust_size);
-
-		/*
-		 * On FAT32 we must fetch the FAT entries for the next
-		 * root directory clusters when a cluster has been
-		 * completely processed.
-		 */
-		++buffer_blk_cnt;
-		int rootdir_end = 0;
-		if (mydata->fatsize == 32) {
-			if (buffer_blk_cnt == mydata->clust_size) {
-				int nxtsect = 0;
-				int nxt_clust = 0;
-
-				nxt_clust = get_fatent(mydata, root_cluster);
-				rootdir_end = CHECK_CLUST(nxt_clust, 32);
-
-				nxtsect = mydata->data_begin +
-					(nxt_clust * mydata->clust_size);
-
-				root_cluster = nxt_clust;
-
-				cursect = nxtsect;
-				buffer_blk_cnt = 0;
-			}
-		} else {
-			if (buffer_blk_cnt == PREFETCH_BLOCKS)
-				buffer_blk_cnt = 0;
-
-			rootdir_end = (++cursect - mydata->rootdir_sect >=
-				       rootdir_size);
-		}
-
-		/* If end of rootdir reached */
-		if (rootdir_end) {
-			if (dols == LS_ROOT) {
-				printf("\n%d file(s), %d dir(s)\n\n",
-				       files, dirs);
-				*size = 0;
-			}
-			goto exit;
-		}
-	}
-rootdir_done:
-
-	firsttime = 1;
-
-	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.
-		 */
-		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;
+		}
+		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)
-- 
2.13.0