Skip to contentSkip to conversation

fs/fat: cleanups + readdir implementation

PATCH New Rob Clark 4 participants 20 messages Aug 13, 2017 – Aug 14, 2017

Subsystems (2)
lore
Share & tools
lore

Rob Clark

author
#0 lore raw

cover fs/fat: cleanups + readdir implementation

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.

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

 fs/fat/Makefile    |    4 -
 fs/fat/fat.c       | 1019 +++++++++++++++++++++-------------------------------
 fs/fat/fat_write.c |    4 +-
 fs/fat/file.c      |  183 ----------
 fs/fs.c            |  119 +++++-
 include/fat.h      |   35 +-
 include/fs.h       |   55 +++
 7 files changed, 590 insertions(+), 829 deletions(-)
 delete mode 100644 fs/fat/file.c
Signature · 1 line
2.13.0
author
#1 lore raw

PATCH 1/8 fs/fat: split out helper to init fsdata

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

---
 fs/fat/fat.c  | 73 +++++++++++++++++++++++++++++++++++------------------------
 include/fat.h |  1 +
 2 files changed, 44 insertions(+), 30 deletions(-)
fs/fat/fat.c +43 -30
index 9ad18f96ff..eb3792d4c1 100644
@@ -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++;
include/fat.h +1 -0
index 71879f01ca..6d3fc8e4a6 100644
@@ -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);
Signature · 1 line
2.13.0
author
#2 lore raw

PATCH 2/8 fs/fat: introduce new director iterators

In reply to Rob Clark

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

---
 fs/fat/fat.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 326 insertions(+)
fs/fat/fat.c +326 -0
index eb3792d4c1..69fa7f4cab 100644
@@ -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 = 0;
+	}
+	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)
 {
Signature · 1 line
2.13.0
author
#3 lore raw

PATCH 3/8 fat/fs: convert to directory iterators

In reply to Rob Clark

And drop a whole lot of ugly code!

---
 fs/fat/fat.c  | 723 ++++++----------------------------------------------------
 include/fat.h |   6 -
 2 files changed, 75 insertions(+), 654 deletions(-)
fs/fat/fat.c +75 -648
index 69fa7f4cab..a50a10ba47 100644
@@ -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)
include/fat.h +0 -6
index 6d3fc8e4a6..3e7ab9ea8d 100644
@@ -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)
Signature · 1 line
2.13.0
author
#4 lore raw

PATCH 4/8 fs: add fs_readdir()

In reply to 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.

---
 fs/fs.c      | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/fs.h | 55 +++++++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+)
fs/fs.c +88 -0
index 595ff1fe69..f9ea3480e9 100644
@@ -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,28 @@ 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;
+
+	ret = part_get_info(desc, part, &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 +370,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)
 {
include/fs.h +55 -0

67 lines not on this page · raw message

Signature · 1 line
2.13.0
author
#5 lore raw

PATCH 5/8 fs/fat: implement opendir/readdir/closedir

In reply to Rob Clark

Implement the readdir interface using the directory iterators.

---
 fs/fat/fat.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
fs/fat/fat.c +56 -0
index a50a10ba47..fe5819315b 100644
@@ -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)
 {
 }
Signature · 1 line
2.13.0
author
#6 lore raw

PATCH 6/8 fat/fs: remove a bunch of dead code

In reply to Rob Clark

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

---
 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
fs/fat/Makefile +0 -4
index b60e8486c4..3e2a6b01a8 100644
@@ -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
fs/fat/file.c deleted +0 -183

183 lines not on this page · raw message

include/fat.h +0 -20
index 3e7ab9ea8d..1e8bc44e9a 100644
@@ -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);
 
Signature · 1 line
2.13.0
#7 lore raw

PATCH 6/8 fat/fs: remove a bunch of dead code

Quoted 12 lines · On Sonntag, 13. August 2017 12:45:26 CEST Rob Clark wrote:

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

---
 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

This patch:

Signature · 2 lines
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
author
#8 lore raw

PATCH 7/8 fat/fs: move ls to generic implementation

In reply to 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.

---
 fs/fat/fat.c  | 32 --------------------------------
 fs/fs.c       | 33 +++++++++++++++++++++++++++++++--
 include/fat.h |  5 ++++-
 3 files changed, 35 insertions(+), 35 deletions(-)
fs/fat/fat.c +0 -32
index fe5819315b..08a066d80d 100644
@@ -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;
fs/fs.c +31 -2
index f9ea3480e9..057329d78c 100644
@@ -37,6 +37,33 @@ 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++;
+		}
+	}
+
+	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 +138,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 +148,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
include/fat.h +4 -1
index 1e8bc44e9a..b2d4b952fd 100644
@@ -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_ */
Signature · 1 line
2.13.0
author
#9 lore raw

PATCH 8/8 fs/fat: fix case for FAT shortnames

In reply to 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.

---
 fs/fat/fat.c       | 11 ++++++++---
 fs/fat/fat_write.c |  4 ++--
 include/fat.h      |  3 +++
 3 files changed, 13 insertions(+), 5 deletions(-)
fs/fat/fat.c +8 -3
index 08a066d80d..8e1790a590 100644
@@ -31,7 +31,7 @@ static const int vfat_enabled = 0;
 /*
  * Convert a string to lowercase.
  */
-static void downcase(char *str)
+static void downcase(char *str, unsigned num)
 {
 	while (*str != '\0') {
 		*str = tolower(*str);
@@ -131,10 +131,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 +150,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);
fs/fat/fat_write.c +2 -2
index f6f06289f4..3c8cc36b8a 100644
@@ -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,
include/fat.h +3 -0
index b2d4b952fd..5e4924316a 100644
@@ -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 */
Signature · 1 line
2.13.0
#10 lore raw

PATCH 8/8 fs/fat: fix case for FAT shortnames

Quoted 24 lines · On Sonntag, 13. August 2017 12:45:28 CEST 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.

---
 fs/fat/fat.c       | 11 ++++++++---
 fs/fat/fat_write.c |  4 ++--
 include/fat.h      |  3 +++
 3 files changed, 13 insertions(+), 5 deletions(-)
fs/fat/fat.c +1 -1
index 08a066d80d..8e1790a590 100644
@@ -31,7 +31,7 @@ static const int vfat_enabled = 0;
 /*
  * Convert a string to lowercase.
  */
-static void downcase(char *str)
+static void downcase(char *str, unsigned num)
 {
 	while (*str != '\0') {
 		*str = tolower(*str);

fs/fat/fat.c static void get_name(dir_entry *dirent, char *s_name) · line 131

Quoted 1 line
fs/fat/fat.c +0 -0
@@ -131,10 +131,16 @@ static void get_name(dir_entry *dirent, char *s_name)
 	ptr = s_name;

This change obviously lacks the actual code to handle num, and it would be nice to have:
- a parameter description
- a mention that the len may be larger than strlen(str) if str is NULL terminated.

And probably, "size_t len" instead of "unsigned num".

Regards,

Stefan

Signature · 2 lines
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034     mobile: +49 151 50412019
author
#11 lore raw

PATCH 8/8 fs/fat: fix case for FAT shortnames

Quoted 33 lines · Stefan Bruens
Quoted 24 lines · On Sonntag, 13. August 2017 12:45:28 CEST 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.

---
 fs/fat/fat.c       | 11 ++++++++---
 fs/fat/fat_write.c |  4 ++--
 include/fat.h      |  3 +++
 3 files changed, 13 insertions(+), 5 deletions(-)
fs/fat/fat.c +1 -1
index 08a066d80d..8e1790a590 100644
@@ -31,7 +31,7 @@ static const int vfat_enabled = 0;
 /*
  * Convert a string to lowercase.
  */
-static void downcase(char *str)
+static void downcase(char *str, unsigned num)
 {
      while (*str != '\0') {
              *str = tolower(*str);

fs/fat/fat.c static void get_name(dir_entry *dirent, char *s_name) · line 131

Quoted 1 line
fs/fat/fat.c +0 -0
@@ -131,10 +131,16 @@ static void get_name(dir_entry *dirent, char *s_name)
      ptr = s_name;

This change obviously lacks the actual code to handle num, and it would be nice to have:
- a parameter description
- a mention that the len may be larger than strlen(str) if str is NULL terminated.

Quoted 2 lines

And probably, "size_t len" instead of "unsigned num".

oh, whoops.. how did I miss that.

BR,
-R

author
#12 lore raw

PATCH v2 8/8 fs/fat: fix case for FAT shortnames

In reply to 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.

---
 fs/fat/fat.c       | 17 ++++++++++++-----
 fs/fat/fat_write.c |  4 ++--
 include/fat.h      |  3 +++
 3 files changed, 17 insertions(+), 7 deletions(-)
fs/fat/fat.c +12 -5
index 08a066d80d..0626d4cb8c 100644
@@ -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);
fs/fat/fat_write.c +2 -2
index f6f06289f4..3c8cc36b8a 100644
@@ -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,
include/fat.h +3 -0
index b2d4b952fd..5e4924316a 100644
@@ -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 */
Signature · 1 line
2.13.0
#13 lore raw

cover fs/fat: cleanups + readdir implementation

In reply to Rob Clark
Quoted 20 lines · 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.

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

Quoted 10 lines
 fs/fat/Makefile    |    4 -
 fs/fat/fat.c       | 1019 +++++++++++++++++++++-------------------------------
 fs/fat/fat_write.c |    4 +-
 fs/fat/file.c      |  183 ----------
 fs/fs.c            |  119 +++++-
 include/fat.h      |   35 +-
 include/fs.h       |   55 +++
 7 files changed, 590 insertions(+), 829 deletions(-)
 delete mode 100644 fs/fat/file.c

Hello Rob,

I assume this patch series is meant to be merged before [PATCH v1 00/15] enough UEFI for standard distro boot and partially replaces it. Did I get it right?

Regards

Heinrich

author
#14 lore raw

cover fs/fat: cleanups + readdir implementation

Quoted 31 lines · Heinrich Schuchardt
Quoted 20 lines · 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.

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

Quoted 10 lines
 fs/fat/Makefile    |    4 -
 fs/fat/fat.c       | 1019 +++++++++++++++++++++-------------------------------
 fs/fat/fat_write.c |    4 +-
 fs/fat/file.c      |  183 ----------
 fs/fs.c            |  119 +++++-
 include/fat.h      |   35 +-
 include/fs.h       |   55 +++
 7 files changed, 590 insertions(+), 829 deletions(-)
 delete mode 100644 fs/fat/file.c
Quoted 6 lines

Hello Rob,

I assume this patch series is meant to be merged before [PATCH v1 00/15] enough UEFI for standard distro boot and partially replaces it. Did I get it right?

correct, the branch ordering goes: efi-next -> vsprintf -> readdir -> enough-uefi-for-shim-2

You can find the latest version of the latter, w/ efi_file re-worked for updated fs_readdir API, on my github tree. I'll be re-sending it soon but still have a few review comments to address first.

BR,
-R

#15 lore raw

cover fs/fat: cleanups + readdir implementation

In reply to Rob Clark
Quoted 9 lines · 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.

Did you run test/fs/fs-test.sh and confirm there's at least no regressions? Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170813/86c1736b/attachment.sig>
author
#16 lore raw

cover fs/fat: cleanups + readdir implementation

Quoted 12 lines · Tom Rini
Quoted 9 lines · 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.

Quoted 2 lines

Did you run test/fs/fs-test.sh and confirm there's at least no regressions? Thanks!

I've been having trouble getting fs-test.sh to pass even without my changes, so no..

Not entirely sure if I have newer kernel/etc compared to the last person who ran fs-test.sh, or if there is something on u-boot side which changed without corresponding change in fs-test.sh.

BR,
-R

#17 lore raw

cover fs/fat: cleanups + readdir implementation

Quoted 13 lines · Rob Clark
Quoted 12 lines · Tom Rini
Quoted 9 lines · 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.

Quoted 5 lines
Quoted 2 lines

Did you run test/fs/fs-test.sh and confirm there's at least no regressions? Thanks!

I've been having trouble getting fs-test.sh to pass even without my changes, so no..

Pass, or provide expected results? From the script, we expect:
# Total Summary: TOTAL PASS: 132 TOTAL FAIL: 6

Quoted 3 lines

Not entirely sure if I have newer kernel/etc compared to the last person who ran fs-test.sh, or if there is something on u-boot side which changed without corresponding change in fs-test.sh.

Note that this script it part of my everything-my-lab-isn't-giving-me-fits loop. I'm probably going to split that loop into pure-SW and HW, as the HW part is what's driving me crazy most of the time.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170813/c2742512/attachment.sig>
author
#18 lore raw

cover fs/fat: cleanups + readdir implementation

Quoted 14 lines · Tom Rini
Quoted 13 lines · Rob Clark
Quoted 12 lines · Tom Rini
Quoted 9 lines · 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.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

Did you run test/fs/fs-test.sh and confirm there's at least no regressions? Thanks!

I've been having trouble getting fs-test.sh to pass even without my changes, so no..

Pass, or provide expected results? From the script, we expect:
# Total Summary: TOTAL PASS: 132 TOTAL FAIL: 6

Ok, I was missing special case handling for the "partition is whole disk" case (which I *guess* is only a sandbox thing?).. but with that fixed (and excluding the ext4 tests which have unrelated problems on my setup):

Total Summary: TOTAL PASS: 63 TOTAL FAIL: 6

So if the 6 failed fat tests (below) are "normal", then I guess we're good.. I'll post a v2 shortly.

** Start sandbox/test/fs/fs-test.nonfs.fat.out_clean FAIL - TC12: 1MB write to . - write denied
FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified FAIL - TC13: 1MB read from 1MB.file.w2 - content verified

** Start sandbox/test/fs/fs-test.fs.fat.out_clean FAIL - TC12: 1MB write to . - write denied
FAIL - TC13: 1MB read from ./1MB.file.w2 - content verified FAIL - TC13: 1MB read from 1MB.file.w2 - content verified

BR,
-R

Quoted 11 lines
Quoted 3 lines

Not entirely sure if I have newer kernel/etc compared to the last person who ran fs-test.sh, or if there is something on u-boot side which changed without corresponding change in fs-test.sh.

Note that this script it part of my everything-my-lab-isn't-giving-me-fits loop. I'm probably going to split that loop into pure-SW and HW, as the HW part is what's driving me crazy most of the time.

--
Tom

#19 lore raw

cover fs/fat: cleanups + readdir implementation

Quoted 18 lines · Rob Clark
Quoted 17 lines · Tom Rini
Quoted 16 lines · Rob Clark
Quoted 14 lines · Tom Rini
Quoted 9 lines · 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.

Did you run test/fs/fs-test.sh and confirm there's at least no regressions? Thanks!

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

I've been having trouble getting fs-test.sh to pass even without my changes, so no..

Pass, or provide expected results? From the script, we expect:
# Total Summary: TOTAL PASS: 132 TOTAL FAIL: 6

Ok, I was missing special case handling for the "partition is whole disk" case (which I *guess* is only a sandbox thing?)..

FWIW, nope, I believe some atmel boards utilize whole disk as FAT, IIRC, from when I was trying to come up with a good default value for the ENV_IS_IN_FAT Kconfig options.

[snip]

Quoted 7 lines

fixed (and excluding the ext4 tests which have unrelated problems on my setup):

Total Summary: TOTAL PASS: 63 TOTAL FAIL: 6

So if the 6 failed fat tests (below) are "normal", then I guess we're good.. I'll post a v2 shortly.

Great, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170814/9c253d4e/attachment.sig>