[U-Boot] [PATCH v2 00/23] subject: fs: fat: extend FAT write operations

[U-Boot] [PATCH v2 00/23] subject: fs: fat: extend FAT write operations

[AKASHI Takahiro]

This patch series[1] is an attempt to address FAT write related issues
in an effort of running UEFI SCT (Self-Certification Test) to verify
UEFI support on u-boot.

SCT is a test platform as well as an extentisive collection of test
cases for UEFI specification. It can run all the tests automatically
and save test results to dedicated log files.

AFAIK, what's missing in the current fat file system to safely run
SCT without errors (I don't mean test case failures) are:
* write a file located under sub-directories
* write a file with non-zero offset
* delete a file
* create a directory

This patch series addresses all of them and what's more, it contains
a file system test; "basic" test is directly derived from test/test-fs.sh.
The others, "ext" and "mkdir," are soly for newly added functionality in
this patch series.

Patch#1 to patch#8 are some sort of preparatory ones.
Patch#9 implements write with sub-directories path.
Patch#10 to patch#12 implement write with non-zero offset.
Patch#13 to patch#17 are related to creating a directory.
Patch#18 provides delete, but it doesn't actually delete a file
but instead return 0 with purging a file content, which is good
enough to run SCT for now.
Patch#19 fixes a minor bug in fs-test.sh.
Patch#20 updates a test result summary.
Patch#21 to patch#23 add a file system test in U-boot pytest suite.

I applied this patch series on top of v2018.09-rc along with a couple
of yet-to--be-upstreamed UEFI-related patches, and could successfully
run unmodified SCT[2] on qemu-arm(arm64).

=== future TODO list ===
1. set creation (,access and modifcation) time
2. debug() vs. printf()
3. open()'s parameter checks
4. precisely detect and prevent write() beyond out-of-space
5. create an unique short name from a long file name
6. fat32's fsInfo support
7. remove "sudo" in pytest, if possible, for fat* case

Without (5) or (6), fsck may issue warnings.

[1] https://git.linaro.org/people/takahiro.akashi/u-boot.git fat_write
[2] http://uefi.org/testtools

Changes in v2 (Sep 4, 2018)
* guard the whole content of fat.h with CONFIG_FS_FAT agaisnt a compiler
  warning
* determine total_sect in struct fsdata correctly, depending on fat type
  (12/16 or 32)
* explicitly revert "fs: fat: cannot write to subdirectories"
* add test scripts with pytest, mostly the same as RFC, but
  removing "sudo" for ext4 case

AKASHI Takahiro (23):
  fs: fat: guard the content of include/fat.h
  fs: fat: extend get_fs_info() for write use
  fs: fat: handle "." and ".." of root dir correctly with
    fat_itr_resolve()
  fs: fat: make directory iterator global for write use
  fs: fat: assure iterator's ->dent belongs to ->clust
  Revert "fs: fat: cannot write to subdirectories"
  fs: fat: check and normailze file name
  fs: fat: write returns error code instead of -1
  fs: fat: support write with sub-directory path
  fs: fat: refactor write interface for a file offset
  fs: fat: support write with non-zero offset
  cmd: fat: add offset parameter to fatwrite
  fs: add mkdir interface
  fs: fat: remember the starting cluster number of directory
  fs: fat: support mkdir
  cmd: fat: add fatmkdir command
  efi_loader: file: support creating a directory
  efi_loader: implement a pseudo "file delete"
  fs-test: fix false positive error at Test Case 12
  fs-test: update the test result as of v2018.09
  test/py: convert fs-test.sh to pytest
  test/py: fs: add extended write operation test
  test/py: fs: add fstest/mkdir test

 cmd/fat.c                            |   22 +-
 fs/fat/fat.c                         |  102 +--
 fs/fat/fat_write.c                   | 1063 ++++++++++++++++----------
 fs/fs.c                              |   46 ++
 include/fat.h                        |   40 +
 include/fs.h                         |   10 +
 lib/efi_loader/efi_file.c            |   24 +-
 test/fs/fs-test.sh                   |   24 +-
 test/py/tests/test_fs/conftest.py    |  331 ++++++++
 test/py/tests/test_fs/fstest_defs.py |   13 +
 test/py/tests/test_fs/test_basic.py  |  287 +++++++
 test/py/tests/test_fs/test_ext.py    |  224 ++++++
 test/py/tests/test_fs/test_mkdir.py  |  112 +++
 13 files changed, 1808 insertions(+), 490 deletions(-)
 create mode 100644 test/py/tests/test_fs/conftest.py
 create mode 100644 test/py/tests/test_fs/fstest_defs.py
 create mode 100644 test/py/tests/test_fs/test_basic.py
 create mode 100644 test/py/tests/test_fs/test_ext.py
 create mode 100644 test/py/tests/test_fs/test_mkdir.py

-- 
2.18.0

[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

[AKASHI Takahiro]

The whole content of include/fat.h is private to FAT implementation
and then should be guarded with CONFIG_FS_FAT.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/fat.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/fat.h b/include/fat.h
index 09e142368585..c02839dcb040 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -9,6 +9,8 @@
 #ifndef _FAT_H_
 #define _FAT_H_
 
+#ifdef CONFIG_FS_FAT
+
 #include <asm/byteorder.h>
 #include <fs.h>
 
@@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
 int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
 void fat_closedir(struct fs_dir_stream *dirs);
 void fat_close(void);
+#endif /* CONFIG_FS_FAT */
 #endif /* _FAT_H_ */
-- 
2.18.0

[U-Boot] [PATCH v2 02/23] fs: fat: extend get_fs_info() for write use

[AKASHI Takahiro]

get_fs_info() was introduced in major re-work of read operation by Rob.
We want to reuse this function in write operation by extending it with
additional members in fsdata structure.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat.c  | 7 +++++++
 include/fat.h | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 4efe8a3edaf1..0576658dde09 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -558,10 +558,17 @@ static int get_fs_info(fsdata *mydata)
 
 	if (mydata->fatsize == 32) {
 		mydata->fatlength = bs.fat32_length;
+		mydata->total_sect = bs.total_sect;
 	} else {
 		mydata->fatlength = bs.fat_length;
+		mydata->total_sect = (bs.sectors[1] << 8) + bs.sectors[0];
+		if (!mydata->total_sect)
+			mydata->total_sect = bs.total_sect;
 	}
+	if (!mydata->total_sect) /* unlikely */
+		mydata->total_sect = (u32)cur_part_info.size;
 
+	mydata->fats = bs.fats;
 	mydata->fat_sect = bs.reserved;
 
 	mydata->rootdir_sect = mydata->fat_sect + mydata->fatlength * bs.fats;
diff --git a/include/fat.h b/include/fat.h
index c02839dcb040..127e6622a9b0 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -175,6 +175,8 @@ typedef struct {
 	int	fatbufnum;	/* Used by get_fatent, init to -1 */
 	int	rootdir_size;	/* Size of root dir for non-FAT32 */
 	__u32	root_cluster;	/* First cluster of root dir for FAT32 */
+	u32	total_sect;	/* Number of sectors */
+	int	fats;		/* Number of FATs */
 } fsdata;
 
 static inline u32 clust_to_sect(fsdata *fsdata, u32 clust)
-- 
2.18.0

[U-Boot] [PATCH v2 03/23] fs: fat: handle "." and ".." of root dir correctly with fat_itr_resolve()

[AKASHI Takahiro]

FAT's root directory does not have "." nor ".."
So care must be taken when scanning root directory with fat_itr_resolve().
Without this patch, any file path starting with "." or ".." will not be
resolved at all.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 0576658dde09..eaea9300fd7f 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -931,6 +931,27 @@ static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)
 	while (next[0] && !ISDIRDELIM(next[0]))
 		next++;
 
+	if (itr->is_root) {
+		/* root dir doesn't have "." nor ".." */
+		if ((((next - path) == 1) && !strncmp(path, ".", 1)) ||
+		    (((next - path) == 2) && !strncmp(path, "..", 2))) {
+			/* point back to itself */
+			itr->clust = itr->fsdata->root_cluster;
+			itr->dent = NULL;
+			itr->remaining = 0;
+			itr->last_cluster = 0;
+
+			if (next[0] == 0) {
+				if (type & TYPE_DIR)
+					return 0;
+				else
+					return -ENOENT;
+			}
+
+			return fat_itr_resolve(itr, next, type);
+		}
+	}
+
 	while (fat_itr_next(itr)) {
 		int match = 0;
 		unsigned n = max(strlen(itr->name), (size_t)(next - path));
-- 
2.18.0

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[AKASHI Takahiro]

Directory iterator was introduced in major re-work of read operation by
Rob. We want to use it for write operation extensively as well.
This patch makes relevant functions, as well as iterator definition,
visible outside of fat.c.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat.c  | 39 ++++++---------------------------------
 include/fat.h | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index eaea9300fd7f..0574af0c0011 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -638,25 +638,6 @@ static int get_fs_info(fsdata *mydata)
  * For more complete example, see fat_itr_resolve()
  */
 
-typedef struct {
-	fsdata    *fsdata;        /* filesystem parameters */
-	unsigned   clust;         /* current cluster */
-	int        last_cluster;  /* set once we've read last cluster */
-	int        is_root;       /* is iterator at root directory */
-	int        remaining;     /* remaining dent's in current cluster */
-
-	/* current iterator position values: */
-	dir_entry *dent;          /* current directory entry */
-	char       l_name[VFAT_MAXLEN_BYTES];    /* long (vfat) name */
-	char       s_name[14];    /* short 8.3 name */
-	char      *name;          /* l_name if there is one, else s_name */
-
-	/* storage for current cluster in memory: */
-	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
@@ -665,7 +646,7 @@ static int fat_itr_isdir(fat_itr *itr);
  * @fsdata: filesystem data for the partition
  * @return 0 on success, else -errno
  */
-static int fat_itr_root(fat_itr *itr, fsdata *fsdata)
+int fat_itr_root(fat_itr *itr, fsdata *fsdata)
 {
 	if (get_fs_info(fsdata))
 		return -ENXIO;
@@ -697,7 +678,7 @@ static int fat_itr_root(fat_itr *itr, fsdata *fsdata)
  * @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)
+void fat_itr_child(fat_itr *itr, fat_itr *parent)
 {
 	fsdata *mydata = parent->fsdata;  /* for silly macros */
 	unsigned clustnum = START(parent->dent);
@@ -717,7 +698,7 @@ static void fat_itr_child(fat_itr *itr, fat_itr *parent)
 	itr->last_cluster = 0;
 }
 
-static void *next_cluster(fat_itr *itr)
+void *next_cluster(fat_itr *itr)
 {
 	fsdata *mydata = itr->fsdata;  /* for silly macros */
 	int ret;
@@ -838,7 +819,7 @@ static dir_entry *extract_vfat_name(fat_itr *itr)
  * @return boolean, 1 if success or 0 if no more entries in the
  *    current directory
  */
-static int fat_itr_next(fat_itr *itr)
+int fat_itr_next(fat_itr *itr)
 {
 	dir_entry *dent;
 
@@ -883,19 +864,11 @@ static int fat_itr_next(fat_itr *itr)
  * @itr: the iterator
  * @return true if cursor is at a directory
  */
-static int fat_itr_isdir(fat_itr *itr)
+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.
@@ -911,7 +884,7 @@ static int fat_itr_isdir(fat_itr *itr)
  * @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)
+int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)
 {
 	const char *next;
 
diff --git a/include/fat.h b/include/fat.h
index 127e6622a9b0..d86eb5a11576 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -189,6 +189,38 @@ static inline u32 sect_to_clust(fsdata *fsdata, int sect)
 	return (sect - fsdata->data_begin) / fsdata->clust_size;
 }
 
+/*
+ * Directory iterator
+ */
+
+#define TYPE_FILE 0x1
+#define TYPE_DIR  0x2
+#define TYPE_ANY  (TYPE_FILE | TYPE_DIR)
+
+typedef struct {
+	fsdata    *fsdata;        /* filesystem parameters */
+	unsigned   clust;         /* current cluster */
+	int        last_cluster;  /* set once we've read last cluster */
+	int        is_root;       /* is iterator at root directory */
+	int        remaining;     /* remaining dent's in current cluster */
+
+	/* current iterator position values: */
+	dir_entry *dent;          /* current directory entry */
+	char       l_name[VFAT_MAXLEN_BYTES];    /* long (vfat) name */
+	char       s_name[14];    /* short 8.3 name */
+	char      *name;          /* l_name if there is one, else s_name */
+
+	/* storage for current cluster in memory: */
+	u8         block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
+} fat_itr;
+
+int fat_itr_root(fat_itr *itr, fsdata *fsdata);
+void fat_itr_child(fat_itr *itr, fat_itr *parent);
+void *next_cluster(fat_itr *itr);
+int fat_itr_next(fat_itr *itr);
+int fat_itr_isdir(fat_itr *itr);
+int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type);
+
 int file_fat_detectfs(void);
 int fat_exists(const char *filename);
 int fat_size(const char *filename, loff_t *size);
-- 
2.18.0

[U-Boot] [PATCH v2 05/23] fs: fat: assure iterator's ->dent belongs to ->clust

[AKASHI Takahiro]

In my attempt to re-work write operation, it was revealed that iterator's
"clust" does not always point to a cluster to which a current directory
entry ("dent") belongs.
This patch assures that it is always true by adding "next_clust" which is
used solely for dereferencing a cluster chain.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat.c  | 24 ++++++++++++++++--------
 include/fat.h |  1 +
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 0574af0c0011..1e414380d527 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -653,6 +653,7 @@ int fat_itr_root(fat_itr *itr, fsdata *fsdata)
 
 	itr->fsdata = fsdata;
 	itr->clust = fsdata->root_cluster;
+	itr->next_clust = fsdata->root_cluster;
 	itr->dent = NULL;
 	itr->remaining = 0;
 	itr->last_cluster = 0;
@@ -688,9 +689,11 @@ void fat_itr_child(fat_itr *itr, fat_itr *parent)
 	itr->fsdata = parent->fsdata;
 	if (clustnum > 0) {
 		itr->clust = clustnum;
+		itr->next_clust = clustnum;
 		itr->is_root = 0;
 	} else {
 		itr->clust = parent->fsdata->root_cluster;
+		itr->next_clust = parent->fsdata->root_cluster;
 		itr->is_root = 1;
 	}
 	itr->dent = NULL;
@@ -708,7 +711,7 @@ void *next_cluster(fat_itr *itr)
 	if (itr->last_cluster)
 		return NULL;
 
-	sect = clust_to_sect(itr->fsdata, itr->clust);
+	sect = clust_to_sect(itr->fsdata, itr->next_clust);
 
 	debug("FAT read(sect=%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n",
 	      sect, itr->fsdata->clust_size, DIRENTSPERBLOCK);
@@ -729,18 +732,19 @@ void *next_cluster(fat_itr *itr)
 		return NULL;
 	}
 
+	itr->clust = itr->next_clust;
 	if (itr->is_root && itr->fsdata->fatsize != 32) {
-		itr->clust++;
-		sect = clust_to_sect(itr->fsdata, itr->clust);
+		itr->next_clust++;
+		sect = clust_to_sect(itr->fsdata, itr->next_clust);
 		if (sect - itr->fsdata->rootdir_sect >=
 		    itr->fsdata->rootdir_size) {
-			debug("cursect: 0x%x\n", itr->clust);
+			debug("nextclust: 0x%x\n", itr->next_clust);
 			itr->last_cluster = 1;
 		}
 	} else {
-		itr->clust = get_fatent(itr->fsdata, itr->clust);
-		if (CHECK_CLUST(itr->clust, itr->fsdata->fatsize)) {
-			debug("cursect: 0x%x\n", itr->clust);
+		itr->next_clust = get_fatent(itr->fsdata, itr->next_clust);
+		if (CHECK_CLUST(itr->next_clust, itr->fsdata->fatsize)) {
+			debug("nextclust: 0x%x\n", itr->next_clust);
 			itr->last_cluster = 1;
 		}
 	}
@@ -756,8 +760,11 @@ static dir_entry *next_dent(fat_itr *itr)
 			itr->fsdata->clust_size;
 
 		/* have we reached the last cluster? */
-		if (!dent)
+		if (!dent) {
+			/* a sign for no more entries left */
+			itr->dent = NULL;
 			return NULL;
+		}
 
 		itr->remaining = nbytes / sizeof(dir_entry) - 1;
 		itr->dent = dent;
@@ -910,6 +917,7 @@ int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type)
 		    (((next - path) == 2) && !strncmp(path, "..", 2))) {
 			/* point back to itself */
 			itr->clust = itr->fsdata->root_cluster;
+			itr->next_clust = itr->fsdata->root_cluster;
 			itr->dent = NULL;
 			itr->remaining = 0;
 			itr->last_cluster = 0;
diff --git a/include/fat.h b/include/fat.h
index d86eb5a11576..9b114e3cbda4 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -200,6 +200,7 @@ static inline u32 sect_to_clust(fsdata *fsdata, int sect)
 typedef struct {
 	fsdata    *fsdata;        /* filesystem parameters */
 	unsigned   clust;         /* current cluster */
+	unsigned   next_clust;    /* next cluster if remaining == 0 */
 	int        last_cluster;  /* set once we've read last cluster */
 	int        is_root;       /* is iterator at root directory */
 	int        remaining;     /* remaining dent's in current cluster */
-- 
2.18.0

[U-Boot] [PATCH v2 06/23] Revert "fs: fat: cannot write to subdirectories"

[AKASHI Takahiro]

This reverts commit 0dc1bfb7302d220a48364263d5632d6d572b069b.
The succeeding patch series will supersede it.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 27e0ff66966c..3b77557b3ede 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -909,11 +909,9 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	volume_info volinfo;
 	fsdata datablock;
 	fsdata *mydata = &datablock;
-	int cursect, i;
+	int cursect;
 	int ret = -1, name_len;
 	char l_filename[VFAT_MAXLEN_BYTES];
-	char bad[2] = " ";
-	const char illegal[] = "<>:\"/\\|?*";
 
 	*actwrite = size;
 	dir_curclust = 0;
@@ -973,18 +971,6 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	}
 	dentptr = (dir_entry *) do_fat_read_at_block;
 
-	/* Strip leading (back-)slashes */
-	while ISDIRDELIM(*filename)
-		++filename;
-	/* Check that the filename is valid */
-	for (i = 0; i < strlen(illegal); ++i) {
-		*bad = illegal[i];
-		if (strstr(filename, bad)) {
-			printf("FAT: illegal filename (%s)\n", filename);
-			return -1;
-		}
-	}
-
 	name_len = strlen(filename);
 	if (name_len >= VFAT_MAXLEN_BYTES)
 		name_len = VFAT_MAXLEN_BYTES - 1;
-- 
2.18.0

[U-Boot] [PATCH v2 07/23] fs: fat: check and normailze file name

[AKASHI Takahiro]

FAT file system's long file name support is a bit complicated and has some
restrictions on its naming. We should be careful about it especially for
write as it may easily end up with wrong file system.

normalize_longname() check for the rules and normalize a file name
if necessary. Please note, however, that this function is yet to be
extended to fully comply with the standard.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 52 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 3b77557b3ede..6c715a70f447 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -899,6 +899,44 @@ static dir_entry *find_directory_entry(fsdata *mydata, int startsect,
 	return NULL;
 }
 
+static int normalize_longname(char *l_filename, const char *filename)
+{
+	const char *p, legal[] = "!#$%&\'()-.@^`_{}~";
+	char c;
+	int name_len;
+
+	/* Check that the filename is valid */
+	for (p = filename; p < filename + strlen(filename); p++) {
+		c = *p;
+
+		if (('0' <= c) && (c <= '9'))
+			continue;
+		if (('A' <= c) && (c <= 'Z'))
+			continue;
+		if (('a' <= c) && (c <= 'z'))
+			continue;
+		if (strchr(legal, c))
+			continue;
+		/* extended code */
+		if ((0x80 <= c) && (c <= 0xff))
+			continue;
+
+		return -1;
+	}
+
+	/* Normalize it */
+	name_len = strlen(filename);
+	if (name_len >= VFAT_MAXLEN_BYTES)
+		/* should return an error? */
+		name_len = VFAT_MAXLEN_BYTES - 1;
+
+	memcpy(l_filename, filename, name_len);
+	l_filename[name_len] = 0; /* terminate the string */
+	downcase(l_filename, INT_MAX);
+
+	return 0;
+}
+
 static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			loff_t *actwrite)
 {
@@ -910,7 +948,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	fsdata datablock;
 	fsdata *mydata = &datablock;
 	int cursect;
-	int ret = -1, name_len;
+	int ret = -1;
 	char l_filename[VFAT_MAXLEN_BYTES];
 
 	*actwrite = size;
@@ -971,13 +1009,11 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	}
 	dentptr = (dir_entry *) do_fat_read_at_block;
 
-	name_len = strlen(filename);
-	if (name_len >= VFAT_MAXLEN_BYTES)
-		name_len = VFAT_MAXLEN_BYTES - 1;
-
-	memcpy(l_filename, filename, name_len);
-	l_filename[name_len] = 0; /* terminate the string */
-	downcase(l_filename, INT_MAX);
+	if (normalize_longname(l_filename, filename)) {
+		printf("FAT: illegal filename (%s)\n", filename);
+		ret = -EINVAL;
+		goto exit;
+	}
 
 	startsect = mydata->rootdir_sect;
 	retdent = find_directory_entry(mydata, startsect,
-- 
2.18.0

[U-Boot] [PATCH v2 08/23] fs: fat: write returns error code instead of -1

[AKASHI Takahiro]

It would be good that FAT write function return error code instead of
just returning -1 as fat_read_file() does.
This patch attempts to address this issue although it is 'best effort
(or estimate)' for now.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 6c715a70f447..1e4f5af9106d 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -956,7 +956,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 
 	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
 		debug("error: reading boot sector\n");
-		return -1;
+		return -EIO;
 	}
 
 	total_sector = bs.total_sect;
@@ -997,7 +997,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	mydata->fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE);
 	if (mydata->fatbuf == NULL) {
 		debug("Error: allocating memory\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	if (disk_read(cursect,
@@ -1005,6 +1005,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 		(mydata->clust_size) :
 		PREFETCH_BLOCKS, do_fat_read_at_block) < 0) {
 		debug("Error: reading rootdir block\n");
+		ret = -EIO;
 		goto exit;
 	}
 	dentptr = (dir_entry *) do_fat_read_at_block;
@@ -1029,6 +1030,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 							size);
 				if (ret) {
 					printf("Error: %llu overflow\n", size);
+					ret = -ENOSPC;
 					goto exit;
 				}
 			}
@@ -1036,6 +1038,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			ret = clear_fatent(mydata, start_cluster);
 			if (ret) {
 				printf("Error: clearing FAT entries\n");
+				ret = -EIO;
 				goto exit;
 			}
 
@@ -1045,12 +1048,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			ret = start_cluster = find_empty_cluster(mydata);
 			if (ret < 0) {
 				printf("Error: finding empty cluster\n");
+				ret = -ENOSPC;
 				goto exit;
 			}
 
 			ret = check_overflow(mydata, start_cluster, size);
 			if (ret) {
 				printf("Error: %llu overflow\n", size);
+				ret = -ENOSPC;
 				goto exit;
 			}
 
@@ -1065,12 +1070,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			ret = start_cluster = find_empty_cluster(mydata);
 			if (ret < 0) {
 				printf("Error: finding empty cluster\n");
+				ret = -ENOSPC;
 				goto exit;
 			}
 
 			ret = check_overflow(mydata, start_cluster, size);
 			if (ret) {
 				printf("Error: %llu overflow\n", size);
+				ret = -ENOSPC;
 				goto exit;
 			}
 		} else {
@@ -1087,6 +1094,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	ret = set_contents(mydata, retdent, buffer, size, actwrite);
 	if (ret < 0) {
 		printf("Error: writing contents\n");
+		ret = -EIO;
 		goto exit;
 	}
 	debug("attempt to write 0x%llx bytes\n", *actwrite);
@@ -1095,14 +1103,17 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	ret = flush_dirty_fat_buffer(mydata);
 	if (ret) {
 		printf("Error: flush fat buffer\n");
+		ret = -EIO;
 		goto exit;
 	}
 
 	/* Write directory table to device */
 	ret = set_cluster(mydata, dir_curclust, get_dentfromdir_block,
 			mydata->clust_size * mydata->sect_size);
-	if (ret)
+	if (ret) {
 		printf("Error: writing directory entry\n");
+		ret = -EIO;
+	}
 
 exit:
 	free(mydata->fatbuf);
@@ -1114,7 +1125,7 @@ int file_fat_write(const char *filename, void *buffer, loff_t offset,
 {
 	if (offset != 0) {
 		printf("Error: non zero offset is currently not supported.\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	printf("writing %s\n", filename);
-- 
2.18.0

[U-Boot] [PATCH v2 09/23] fs: fat: support write with sub-directory path

[AKASHI Takahiro]

In this patch, write implementation is overhauled and rewritten by
making full use of directory iterator. The obvious bonus is that we are
now able to write to a file with a directory path, like /A/B/C/FILE.

Please note that, as there is no notion of "current directory" on u-boot,
a file name specified must contain an absolute directory path. Otherwise,
"/" (root directory) is assumed.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat.c       |   9 -
 fs/fat/fat_write.c | 467 +++++++++++++++------------------------------
 2 files changed, 155 insertions(+), 321 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 1e414380d527..50c2d4f3c33d 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -464,15 +464,6 @@ static __u8 mkcksum(const char name[8], const char ext[3])
 	return ret;
 }
 
-/*
- * TODO these should go away once fat_write is reworked to use the
- * directory iterator
- */
-__u8 get_dentfromdir_block[MAX_CLUSTSIZE]
-	__aligned(ARCH_DMA_MINALIGN);
-__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
-	__aligned(ARCH_DMA_MINALIGN);
-
 /*
  * Read boot sector and volume info from a FAT filesystem
  */
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 1e4f5af9106d..cc55c94f1b72 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -99,7 +99,6 @@ static void set_name(dir_entry *dirent, const char *filename)
 	debug("ext : %s\n", dirent->ext);
 }
 
-static __u8 num_of_fats;
 /*
  * Write fat buffer into block device
  */
@@ -128,7 +127,7 @@ static int flush_dirty_fat_buffer(fsdata *mydata)
 		return -1;
 	}
 
-	if (num_of_fats == 2) {
+	if (mydata->fats == 2) {
 		/* Update corresponding second FAT blocks */
 		startblock += mydata->fatlength;
 		if (disk_write(startblock, getsize, bufptr) < 0) {
@@ -210,15 +209,14 @@ name11_12:
 	return 1;
 }
 
-static int is_next_clust(fsdata *mydata, dir_entry *dentptr);
-static void flush_dir_table(fsdata *mydata, dir_entry **dentptr);
+static int flush_dir_table(fat_itr *itr);
 
 /*
  * Fill dir_slot entries with appropriate name, id, and attr
- * The real directory entry is returned by 'dentptr'
+ * 'itr' will point to a next entry
  */
-static void
-fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)
+static int
+fill_dir_slot(fat_itr *itr, const char *l_name)
 {
 	__u8 temp_dir_slot_buffer[MAX_LFN_SLOT * sizeof(dir_slot)];
 	dir_slot *slotptr = (dir_slot *)temp_dir_slot_buffer;
@@ -226,7 +224,7 @@ fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)
 	int idx = 0, ret;
 
 	/* Get short file name checksum value */
-	checksum = mkcksum((*dentptr)->name, (*dentptr)->ext);
+	checksum = mkcksum(itr->dent->name, itr->dent->ext);
 
 	do {
 		memset(slotptr, 0x00, sizeof(dir_slot));
@@ -241,120 +239,21 @@ fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)
 	slotptr->id |= LAST_LONG_ENTRY_MASK;
 
 	while (counter >= 1) {
-		if (is_next_clust(mydata, *dentptr)) {
-			/* A new cluster is allocated for directory table */
-			flush_dir_table(mydata, dentptr);
-		}
-		memcpy(*dentptr, slotptr, sizeof(dir_slot));
-		(*dentptr)++;
+		memcpy(itr->dent, slotptr, sizeof(dir_slot));
 		slotptr--;
 		counter--;
-	}
-
-	if (is_next_clust(mydata, *dentptr)) {
-		/* A new cluster is allocated for directory table */
-		flush_dir_table(mydata, dentptr);
-	}
-}
-
-static __u32 dir_curclust;
-
-/*
- * 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'
- * If additional adjacent cluster for directory entries is read into memory,
- * then 'get_contents_vfatname_block' is copied into 'get_dentfromdir_block' and
- * the location of the real directory entry is returned by 'retdent'
- * Return 0 on success, -1 otherwise.
- */
-static int
-get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
-	      dir_entry **retdent, char *l_name)
-{
-	dir_entry *realdent;
-	dir_slot *slotptr = (dir_slot *)(*retdent);
-	dir_slot *slotptr2 = NULL;
-	__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, cur_position = 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) {
-		if (curclust == 0)
-			return -1;
-		curclust = get_fatent(mydata, dir_curclust);
-		if (CHECK_CLUST(curclust, mydata->fatsize)) {
-			debug("curclust: 0x%x\n", curclust);
-			printf("Invalid FAT entry\n");
-			return -1;
-		}
-
-		dir_curclust = curclust;
-
-		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)
+		if (!fat_itr_next(itr))
+			if (!itr->dent && !itr->is_root && flush_dir_table(itr))
 				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, INT_MAX);
-
-	/* Return the real directory entry */
-	*retdent = realdent;
-
-	if (slotptr2) {
-		memcpy(get_dentfromdir_block, get_contents_vfatname_block,
-			mydata->clust_size * mydata->sect_size);
-		cur_position = (__u8 *)realdent - get_contents_vfatname_block;
-		*retdent = (dir_entry *) &get_dentfromdir_block[cur_position];
-	}
+	if (!itr->dent && !itr->is_root)
+		/*
+		 * don't care return value here because we have already
+		 * finished completing an entry with name, only ending up
+		 * no more entry left
+		 */
+		flush_dir_table(itr);
 
 	return 0;
 }
@@ -569,20 +468,20 @@ static int find_empty_cluster(fsdata *mydata)
 }
 
 /*
- * Write directory entries in 'get_dentfromdir_block' to block device
+ * Write directory entries in itr's buffer to block device
  */
-static void flush_dir_table(fsdata *mydata, dir_entry **dentptr)
+static int flush_dir_table(fat_itr *itr)
 {
+	fsdata *mydata = itr->fsdata;
 	int dir_newclust = 0;
+	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
 
-	if (set_cluster(mydata, dir_curclust,
-		    get_dentfromdir_block,
-		    mydata->clust_size * mydata->sect_size) != 0) {
-		printf("error: wrinting directory entry\n");
-		return;
+	if (set_cluster(mydata, itr->clust, itr->block, bytesperclust) != 0) {
+		printf("error: writing directory entry\n");
+		return -1;
 	}
 	dir_newclust = find_empty_cluster(mydata);
-	set_fatent_value(mydata, dir_curclust, dir_newclust);
+	set_fatent_value(mydata, itr->clust, dir_newclust);
 	if (mydata->fatsize == 32)
 		set_fatent_value(mydata, dir_newclust, 0xffffff8);
 	else if (mydata->fatsize == 16)
@@ -590,15 +489,19 @@ static void flush_dir_table(fsdata *mydata, dir_entry **dentptr)
 	else if (mydata->fatsize == 12)
 		set_fatent_value(mydata, dir_newclust, 0xff8);
 
-	dir_curclust = dir_newclust;
+	itr->clust = dir_newclust;
+	itr->next_clust = dir_newclust;
 
 	if (flush_dirty_fat_buffer(mydata) < 0)
-		return;
+		return -1;
+
+	memset(itr->block, 0x00, bytesperclust);
 
-	memset(get_dentfromdir_block, 0x00,
-		mydata->clust_size * mydata->sect_size);
+	itr->dent = (dir_entry *)itr->block;
+	itr->last_cluster = 1;
+	itr->remaining = bytesperclust / sizeof(dir_entry) - 1;
 
-	*dentptr = (dir_entry *) get_dentfromdir_block;
+	return 0;
 }
 
 /*
@@ -764,139 +667,83 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size)
 	return 0;
 }
 
-/*
- * Check if adding several entries exceed one cluster boundary
- */
-static int is_next_clust(fsdata *mydata, dir_entry *dentptr)
-{
-	int cur_position;
-
-	cur_position = (__u8 *)dentptr - get_dentfromdir_block;
-
-	if (cur_position >= mydata->clust_size * mydata->sect_size)
-		return 1;
-	else
-		return 0;
-}
-
-static dir_entry *empty_dentptr;
 /*
  * Find a directory entry based on filename or start cluster number
  * If the directory entry is not found,
  * the new position for writing a directory entry will be returned
  */
-static dir_entry *find_directory_entry(fsdata *mydata, int startsect,
-	char *filename, dir_entry *retdent, __u32 start)
+static dir_entry *find_directory_entry(fat_itr *itr, char *filename)
 {
-	__u32 curclust = sect_to_clust(mydata, startsect);
-
-	debug("get_dentfromdir: %s\n", filename);
+	int match = 0;
 
-	while (1) {
-		dir_entry *dentptr;
+	while (fat_itr_next(itr)) {
+		/* check both long and short name: */
+		if (!strcasecmp(filename, itr->name))
+			match = 1;
+		else if (itr->name != itr->s_name &&
+			 !strcasecmp(filename, itr->s_name))
+			match = 1;
 
-		int i;
+		if (!match)
+			continue;
 
-		if (get_cluster(mydata, curclust, get_dentfromdir_block,
-			    mydata->clust_size * mydata->sect_size) != 0) {
-			printf("Error: reading directory block\n");
+		if (itr->dent->name[0] == '\0')
 			return NULL;
-		}
-
-		dentptr = (dir_entry *)get_dentfromdir_block;
-
-		dir_curclust = curclust;
-
-		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++;
-				if (is_next_clust(mydata, dentptr))
-					break;
-				continue;
-			}
-			if ((dentptr->attr & ATTR_VOLUME)) {
-				if ((dentptr->attr & ATTR_VFAT) &&
-				    (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) {
-					get_long_file_name(mydata, curclust,
-						     get_dentfromdir_block,
-						     &dentptr, l_name);
-					debug("vfatname: |%s|\n", l_name);
-				} else {
-					/* Volume label or VFAT entry */
-					dentptr++;
-					if (is_next_clust(mydata, dentptr))
-						break;
-					continue;
-				}
-			}
-			if (dentptr->name[0] == 0) {
-				debug("Dentname == NULL - %d\n", i);
-				empty_dentptr = dentptr;
-				return NULL;
-			}
-
-			get_name(dentptr, s_name);
-
-			if (strncasecmp(filename, s_name, sizeof(s_name)) &&
-			    strncasecmp(filename, l_name, sizeof(l_name))) {
-				debug("Mismatch: |%s|%s|\n",
-					s_name, l_name);
-				dentptr++;
-				if (is_next_clust(mydata, dentptr))
-					break;
-				continue;
-			}
+		else
+			return itr->dent;
+	}
 
-			memcpy(retdent, dentptr, sizeof(dir_entry));
+	if (!itr->dent && !itr->is_root && flush_dir_table(itr))
+		/* indicate that allocating dent failed */
+		itr->dent = NULL;
 
-			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 NULL;
+}
 
-			return dentptr;
+static int split_filename(char *filename, char **dirname, char **basename)
+{
+	char *p, *last_slash, *last_slash_cont;
+
+again:
+	p = filename;
+	last_slash = NULL;
+	last_slash_cont = NULL;
+	while (*p) {
+		if (ISDIRDELIM(*p)) {
+			last_slash = p;
+			last_slash_cont = p;
+			/* continuous slashes */
+			while (ISDIRDELIM(*p))
+				last_slash_cont = p++;
+			if (!*p)
+				break;
 		}
+		p++;
+	}
 
-		/*
-		 * In FAT16/12, the root dir is locate before data area, shows
-		 * in following:
-		 * -------------------------------------------------------------
-		 * | Boot | FAT1 & 2 | Root dir | Data (start from cluster #2) |
-		 * -------------------------------------------------------------
-		 *
-		 * As a result if curclust is in Root dir, it is a negative
-		 * number or 0, 1.
-		 *
-		 */
-		if (mydata->fatsize != 32 && (int)curclust <= 1) {
-			/* Current clust is in root dir, set to next clust */
-			curclust++;
-			if ((int)curclust <= 1)
-				continue;	/* continue to find */
-
-			/* Reach the end of root dir */
-			empty_dentptr = dentptr;
-			return NULL;
+	if (last_slash) {
+		if (last_slash_cont == (filename + strlen(filename) - 1)) {
+			/* remove trailing slashes */
+			*last_slash = '\0';
+			goto again;
 		}
 
-		curclust = get_fatent(mydata, dir_curclust);
-		if (IS_LAST_CLUST(curclust, mydata->fatsize)) {
-			empty_dentptr = dentptr;
-			return NULL;
-		}
-		if (CHECK_CLUST(curclust, mydata->fatsize)) {
-			debug("curclust: 0x%x\n", curclust);
-			debug("Invalid FAT entry\n");
-			return NULL;
+		if (last_slash == filename) {
+			/* avoid ""(null) directory */
+			*dirname = "/";
+		} else {
+			*last_slash = '\0';
+			*dirname = filename;
 		}
+
+		*last_slash_cont = '\0';
+		*basename = last_slash_cont + 1;
+	} else {
+		*dirname = "/"; /* root by default */
+		*basename = filename;
 	}
 
-	return NULL;
+	return 0;
 }
 
 static int normalize_longname(char *l_filename, const char *filename)
@@ -940,86 +787,58 @@ static int normalize_longname(char *l_filename, const char *filename)
 static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			loff_t *actwrite)
 {
-	dir_entry *dentptr, *retdent;
-	__u32 startsect;
+	dir_entry *retdent;
 	__u32 start_cluster;
-	boot_sector bs;
-	volume_info volinfo;
-	fsdata datablock;
+	fsdata datablock = { .fatbuf = NULL, };
 	fsdata *mydata = &datablock;
-	int cursect;
+	fat_itr *itr = NULL;
 	int ret = -1;
+	char *filename_copy, *parent, *basename;
 	char l_filename[VFAT_MAXLEN_BYTES];
 
-	*actwrite = size;
-	dir_curclust = 0;
+	filename_copy = strdup(filename);
+	if (!filename_copy)
+		return -ENOMEM;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
-		debug("error: reading boot sector\n");
-		return -EIO;
+	split_filename(filename_copy, &parent, &basename);
+	if (!strlen(basename)) {
+		ret = -EINVAL;
+		goto exit;
 	}
 
-	total_sector = bs.total_sect;
-	if (total_sector == 0)
-		total_sector = (int)cur_part_info.size; /* cast of lbaint_t */
-
-	if (mydata->fatsize == 32)
-		mydata->fatlength = bs.fat32_length;
-	else
-		mydata->fatlength = bs.fat_length;
-
-	mydata->fat_sect = bs.reserved;
-
-	cursect = mydata->rootdir_sect
-		= mydata->fat_sect + mydata->fatlength * bs.fats;
-	num_of_fats = bs.fats;
-
-	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
-	mydata->clust_size = bs.cluster_size;
-
-	if (mydata->fatsize == 32) {
-		mydata->data_begin = mydata->rootdir_sect -
-					(mydata->clust_size * 2);
-	} else {
-		int rootdir_size;
-
-		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->clust_size * 2);
+	filename = basename;
+	if (normalize_longname(l_filename, filename)) {
+		printf("FAT: illegal filename (%s)\n", filename);
+		ret = -EINVAL;
+		goto exit;
 	}
 
-	mydata->fatbufnum = -1;
-	mydata->fat_dirty = 0;
-	mydata->fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE);
-	if (mydata->fatbuf == NULL) {
-		debug("Error: allocating memory\n");
-		return -ENOMEM;
+	itr = malloc_cache_aligned(sizeof(fat_itr));
+	if (!itr) {
+		ret = -ENOMEM;
+		goto exit;
 	}
 
-	if (disk_read(cursect,
-		(mydata->fatsize == 32) ?
-		(mydata->clust_size) :
-		PREFETCH_BLOCKS, do_fat_read_at_block) < 0) {
-		debug("Error: reading rootdir block\n");
-		ret = -EIO;
+	ret = fat_itr_root(itr, &datablock);
+	if (ret)
 		goto exit;
-	}
-	dentptr = (dir_entry *) do_fat_read_at_block;
 
-	if (normalize_longname(l_filename, filename)) {
-		printf("FAT: illegal filename (%s)\n", filename);
-		ret = -EINVAL;
+	total_sector = datablock.total_sect;
+
+	ret = fat_itr_resolve(itr, parent, TYPE_DIR);
+	if (ret) {
+		printf("%s: doesn't exist (%d)\n", parent, ret);
 		goto exit;
 	}
 
-	startsect = mydata->rootdir_sect;
-	retdent = find_directory_entry(mydata, startsect,
-				l_filename, dentptr, 0);
+	retdent = find_directory_entry(itr, l_filename);
+
 	if (retdent) {
+		if (fat_itr_isdir(itr)) {
+			ret = -EISDIR;
+			goto exit;
+		}
+
 		/* Update file size and start_cluster in a directory entry */
 		retdent->size = cpu_to_le32(size);
 		start_cluster = START(retdent);
@@ -1062,9 +881,31 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			set_start_cluster(mydata, retdent, start_cluster);
 		}
 	} else {
+		/* Create a new file */
+
+		if (itr->is_root) {
+			/* root dir cannot have "." or ".." */
+			if (!strcmp(l_filename, ".") ||
+			    !strcmp(l_filename, "..")) {
+				ret = -EINVAL;
+				goto exit;
+			}
+		}
+
+		if (!itr->dent) {
+			printf("Error: allocating new dir entry\n");
+			ret = -EIO;
+			goto exit;
+		}
+
+		memset(itr->dent, 0, sizeof(*itr->dent));
+
 		/* Set short name to set alias checksum field in dir_slot */
-		set_name(empty_dentptr, filename);
-		fill_dir_slot(mydata, &empty_dentptr, filename);
+		set_name(itr->dent, filename);
+		if (fill_dir_slot(itr, filename)) {
+			ret = -EIO;
+			goto exit;
+		}
 
 		if (size) {
 			ret = start_cluster = find_empty_cluster(mydata);
@@ -1085,10 +926,10 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 		}
 
 		/* Set attribute as archieve for regular file */
-		fill_dentry(mydata, empty_dentptr, filename,
-			start_cluster, size, 0x20);
+		fill_dentry(itr->fsdata, itr->dent, filename,
+			    start_cluster, size, 0x20);
 
-		retdent = empty_dentptr;
+		retdent = itr->dent;
 	}
 
 	ret = set_contents(mydata, retdent, buffer, size, actwrite);
@@ -1108,15 +949,17 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	}
 
 	/* Write directory table to device */
-	ret = set_cluster(mydata, dir_curclust, get_dentfromdir_block,
-			mydata->clust_size * mydata->sect_size);
+	ret = set_cluster(mydata, itr->clust, itr->block,
+			  mydata->clust_size * mydata->sect_size);
 	if (ret) {
 		printf("Error: writing directory entry\n");
 		ret = -EIO;
 	}
 
 exit:
+	free(filename_copy);
 	free(mydata->fatbuf);
+	free(itr);
 	return ret;
 }
 
-- 
2.18.0

[U-Boot] [PATCH v2 10/23] fs: fat: refactor write interface for a file offset

[AKASHI Takahiro]

The current write implementation is quite simple: remove existing clusters
and then allocating new ones and filling them with data. This, inevitably,
enforces always writing from the beginning of a file.

As the first step to lift this restriction, fat_file_write() and
set_contents() are modified to accept an additional parameter, file offset
and further re-factored so that, in the next patch, all the necessary code
will be put into set_contents().

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 179 ++++++++++++++++-----------------------------
 1 file changed, 65 insertions(+), 114 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index cc55c94f1b72..5db7830ad43c 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -528,6 +528,42 @@ static int clear_fatent(fsdata *mydata, __u32 entry)
 	return 0;
 }
 
+/*
+ * Set start cluster in directory entry
+ */
+static void set_start_cluster(const fsdata *mydata, dir_entry *dentptr,
+			      __u32 start_cluster)
+{
+	if (mydata->fatsize == 32)
+		dentptr->starthi =
+			cpu_to_le16((start_cluster & 0xffff0000) >> 16);
+	dentptr->start = cpu_to_le16(start_cluster & 0xffff);
+}
+
+/*
+ * Check whether adding a file makes the file system to
+ * exceed the size of the block device
+ * Return -1 when overflow occurs, otherwise return 0
+ */
+static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size)
+{
+	__u32 startsect, sect_num, offset;
+
+	if (clustnum > 0)
+		startsect = clust_to_sect(mydata, clustnum);
+	else
+		startsect = mydata->rootdir_sect;
+
+	sect_num = div_u64_rem(size, mydata->sect_size, &offset);
+
+	if (offset != 0)
+		sect_num++;
+
+	if (startsect + sect_num > total_sector)
+		return -1;
+	return 0;
+}
+
 /*
  * Write at most 'maxsize' bytes from 'buffer' into
  * the file associated with 'dentptr'
@@ -535,29 +571,36 @@ static int clear_fatent(fsdata *mydata, __u32 entry)
  * or return -1 on fatal errors.
  */
 static int
-set_contents(fsdata *mydata, dir_entry *dentptr, __u8 *buffer,
-	      loff_t maxsize, loff_t *gotsize)
+set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer,
+	     loff_t maxsize, loff_t *gotsize)
 {
-	loff_t filesize = FAT2CPU32(dentptr->size);
+	loff_t filesize;
 	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
 	__u32 curclust = START(dentptr);
 	__u32 endclust = 0, newclust = 0;
 	loff_t actsize;
 
 	*gotsize = 0;
-	debug("Filesize: %llu bytes\n", filesize);
-
-	if (maxsize > 0 && filesize > maxsize)
-		filesize = maxsize;
+	filesize = maxsize;
 
 	debug("%llu bytes\n", filesize);
 
-	if (!curclust) {
-		if (filesize) {
-			debug("error: nonempty clusterless file!\n");
+	if (curclust) {
+		/*
+		 * release already-allocated clusters anyway
+		 */
+		if (clear_fatent(mydata, curclust)) {
+			printf("Error: clearing FAT entries\n");
 			return -1;
 		}
-		return 0;
+	}
+
+	curclust = find_empty_cluster(mydata);
+	set_start_cluster(mydata, dentptr, curclust);
+
+	if (check_overflow(mydata, curclust, filesize)) {
+		printf("Error: no space left: %llu\n", filesize);
+		return -1;
 	}
 
 	actsize = bytesperclust;
@@ -568,6 +611,7 @@ set_contents(fsdata *mydata, dir_entry *dentptr, __u8 *buffer,
 			newclust = determine_fatent(mydata, endclust);
 
 			if ((newclust - 1) != endclust)
+				/* write to <curclust..endclust> */
 				goto getit;
 
 			if (CHECK_CLUST(newclust, mydata->fatsize)) {
@@ -614,18 +658,8 @@ getit:
 		actsize = bytesperclust;
 		curclust = endclust = newclust;
 	} while (1);
-}
 
-/*
- * Set start cluster in directory entry
- */
-static void set_start_cluster(const fsdata *mydata, dir_entry *dentptr,
-				__u32 start_cluster)
-{
-	if (mydata->fatsize == 32)
-		dentptr->starthi =
-			cpu_to_le16((start_cluster & 0xffff0000) >> 16);
-	dentptr->start = cpu_to_le16(start_cluster & 0xffff);
+	return 0;
 }
 
 /*
@@ -642,31 +676,6 @@ static void fill_dentry(fsdata *mydata, dir_entry *dentptr,
 	set_name(dentptr, filename);
 }
 
-/*
- * Check whether adding a file makes the file system to
- * exceed the size of the block device
- * Return -1 when overflow occurs, otherwise return 0
- */
-static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size)
-{
-	__u32 startsect, sect_num, offset;
-
-	if (clustnum > 0) {
-		startsect = clust_to_sect(mydata, clustnum);
-	} else {
-		startsect = mydata->rootdir_sect;
-	}
-
-	sect_num = div_u64_rem(size, mydata->sect_size, &offset);
-
-	if (offset != 0)
-		sect_num++;
-
-	if (startsect + sect_num > total_sector)
-		return -1;
-	return 0;
-}
-
 /*
  * Find a directory entry based on filename or start cluster number
  * If the directory entry is not found,
@@ -784,11 +793,10 @@ static int normalize_longname(char *l_filename, const char *filename)
 	return 0;
 }
 
-static int do_fat_write(const char *filename, void *buffer, loff_t size,
-			loff_t *actwrite)
+int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
+		      loff_t size, loff_t *actwrite)
 {
 	dir_entry *retdent;
-	__u32 start_cluster;
 	fsdata datablock = { .fatbuf = NULL, };
 	fsdata *mydata = &datablock;
 	fat_itr *itr = NULL;
@@ -796,6 +804,8 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 	char *filename_copy, *parent, *basename;
 	char l_filename[VFAT_MAXLEN_BYTES];
 
+	debug("writing %s\n", filename);
+
 	filename_copy = strdup(filename);
 	if (!filename_copy)
 		return -ENOMEM;
@@ -839,47 +849,8 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			goto exit;
 		}
 
-		/* Update file size and start_cluster in a directory entry */
-		retdent->size = cpu_to_le32(size);
-		start_cluster = START(retdent);
-
-		if (start_cluster) {
-			if (size) {
-				ret = check_overflow(mydata, start_cluster,
-							size);
-				if (ret) {
-					printf("Error: %llu overflow\n", size);
-					ret = -ENOSPC;
-					goto exit;
-				}
-			}
-
-			ret = clear_fatent(mydata, start_cluster);
-			if (ret) {
-				printf("Error: clearing FAT entries\n");
-				ret = -EIO;
-				goto exit;
-			}
-
-			if (!size)
-				set_start_cluster(mydata, retdent, 0);
-		} else if (size) {
-			ret = start_cluster = find_empty_cluster(mydata);
-			if (ret < 0) {
-				printf("Error: finding empty cluster\n");
-				ret = -ENOSPC;
-				goto exit;
-			}
-
-			ret = check_overflow(mydata, start_cluster, size);
-			if (ret) {
-				printf("Error: %llu overflow\n", size);
-				ret = -ENOSPC;
-				goto exit;
-			}
-
-			set_start_cluster(mydata, retdent, start_cluster);
-		}
+		/* Update file size in a directory entry */
+		retdent->size = cpu_to_le32(pos + size);
 	} else {
 		/* Create a new file */
 
@@ -907,32 +878,13 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size,
 			goto exit;
 		}
 
-		if (size) {
-			ret = start_cluster = find_empty_cluster(mydata);
-			if (ret < 0) {
-				printf("Error: finding empty cluster\n");
-				ret = -ENOSPC;
-				goto exit;
-			}
-
-			ret = check_overflow(mydata, start_cluster, size);
-			if (ret) {
-				printf("Error: %llu overflow\n", size);
-				ret = -ENOSPC;
-				goto exit;
-			}
-		} else {
-			start_cluster = 0;
-		}
-
 		/* Set attribute as archieve for regular file */
-		fill_dentry(itr->fsdata, itr->dent, filename,
-			    start_cluster, size, 0x20);
+		fill_dentry(itr->fsdata, itr->dent, filename, 0, size, 0x20);
 
 		retdent = itr->dent;
 	}
 
-	ret = set_contents(mydata, retdent, buffer, size, actwrite);
+	ret = set_contents(mydata, retdent, pos, buffer, size, actwrite);
 	if (ret < 0) {
 		printf("Error: writing contents\n");
 		ret = -EIO;
@@ -971,6 +923,5 @@ int file_fat_write(const char *filename, void *buffer, loff_t offset,
 		return -EINVAL;
 	}
 
-	printf("writing %s\n", filename);
-	return do_fat_write(filename, buffer, maxsize, actwrite);
+	return file_fat_write_at(filename, offset, buffer, maxsize, actwrite);
 }
-- 
2.18.0

[U-Boot] [PATCH v2 11/23] fs: fat: support write with non-zero offset

[AKASHI Takahiro]

In this patch, all the necessary code for allowing for a file offset
at write is implemented. What plays a major roll here is get_set_cluster(),
which, in contrast to its counterpart, set_cluster(), only operates on
already-allocated clusters, overwriting with data.

So, with a file offset specified, set_contents() seeks and writes data
with set_get_cluster() until the end of a file, and, once it reaches
there, continues writing with set_cluster() for the rest.

Please note that a file will be trimmed as a result of write operation if
write ends before reaching file's end. This is an intended behavior
in order to maintain compatibility with the current interface.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 288 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 273 insertions(+), 15 deletions(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 5db7830ad43c..523abd62c9e9 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -450,6 +450,121 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
 	return 0;
 }
 
+static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
+
+/*
+ * Read and modify data on existing and consecutive cluster blocks
+ */
+static int
+get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer,
+		loff_t size, loff_t *gotsize)
+{
+	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
+	__u32 startsect;
+	loff_t wsize;
+	int clustcount, i, ret;
+
+	*gotsize = 0;
+	if (!size)
+		return 0;
+
+	assert(pos < bytesperclust);
+	startsect = clust_to_sect(mydata, clustnum);
+
+	debug("clustnum: %d, startsect: %d, pos: %lld\n",
+	      clustnum, startsect, pos);
+
+	/* partial write@beginning */
+	if (pos) {
+		wsize = min(bytesperclust - pos, size);
+		ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
+		if (ret != mydata->clust_size) {
+			debug("Error reading data (got %d)\n", ret);
+			return -1;
+		}
+
+		memcpy(tmpbuf_cluster + pos, buffer, wsize);
+		ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
+		if (ret != mydata->clust_size) {
+			debug("Error writing data (got %d)\n", ret);
+			return -1;
+		}
+
+		size -= wsize;
+		buffer += wsize;
+		*gotsize += wsize;
+
+		startsect += mydata->clust_size;
+
+		if (!size)
+			return 0;
+	}
+
+	/* full-cluster write */
+	if (size >= bytesperclust) {
+		clustcount = lldiv(size, bytesperclust);
+
+		if (!((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1))) {
+			wsize = clustcount * bytesperclust;
+			ret = disk_write(startsect,
+					 clustcount * mydata->clust_size,
+					 buffer);
+			if (ret != clustcount * mydata->clust_size) {
+				debug("Error writing data (got %d)\n", ret);
+				return -1;
+			}
+
+			size -= wsize;
+			buffer += wsize;
+			*gotsize += wsize;
+
+			startsect += clustcount * mydata->clust_size;
+		} else {
+			for (i = 0; i < clustcount; i++) {
+				memcpy(tmpbuf_cluster, buffer, bytesperclust);
+				ret = disk_write(startsect,
+						 mydata->clust_size,
+						 tmpbuf_cluster);
+				if (ret != mydata->clust_size) {
+					debug("Error writing data (got %d)\n",
+					      ret);
+					return -1;
+				}
+
+				size -= bytesperclust;
+				buffer += bytesperclust;
+				*gotsize += bytesperclust;
+
+				startsect += mydata->clust_size;
+			}
+		}
+	}
+
+	/* partial write at end */
+	if (size) {
+		wsize = size;
+		ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
+		if (ret != mydata->clust_size) {
+			debug("Error reading data (got %d)\n", ret);
+			return -1;
+		}
+		memcpy(tmpbuf_cluster, buffer, wsize);
+		ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
+		if (ret != mydata->clust_size) {
+			debug("Error writing data (got %d)\n", ret);
+			return -1;
+		}
+
+		size -= wsize;
+		buffer += wsize;
+		*gotsize += wsize;
+	}
+
+	assert(!size);
+
+	return 0;
+}
+
 /*
  * Find the first empty cluster
  */
@@ -578,26 +693,158 @@ set_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos, __u8 *buffer,
 	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
 	__u32 curclust = START(dentptr);
 	__u32 endclust = 0, newclust = 0;
-	loff_t actsize;
+	loff_t cur_pos, offset, actsize, wsize;
 
 	*gotsize = 0;
-	filesize = maxsize;
+	filesize = pos + maxsize;
 
 	debug("%llu bytes\n", filesize);
 
-	if (curclust) {
-		/*
-		 * release already-allocated clusters anyway
-		 */
-		if (clear_fatent(mydata, curclust)) {
-			printf("Error: clearing FAT entries\n");
+	if (!filesize) {
+		if (!curclust)
+			return 0;
+		if (!CHECK_CLUST(curclust, mydata->fatsize) ||
+		    IS_LAST_CLUST(curclust, mydata->fatsize)) {
+			clear_fatent(mydata, curclust);
+			set_start_cluster(mydata, dentptr, 0);
+			return 0;
+		}
+		debug("curclust: 0x%x\n", curclust);
+		debug("Invalid FAT entry\n");
+		return -1;
+	}
+
+	if (!curclust) {
+		assert(pos == 0);
+		goto set_clusters;
+	}
+
+	/* go to cluster at pos */
+	cur_pos = bytesperclust;
+	while (1) {
+		if (pos <= cur_pos)
+			break;
+		if (IS_LAST_CLUST(curclust, mydata->fatsize))
+			break;
+
+		newclust = get_fatent(mydata, curclust);
+		if (!IS_LAST_CLUST(newclust, mydata->fatsize) &&
+		    CHECK_CLUST(newclust, mydata->fatsize)) {
+			debug("curclust: 0x%x\n", curclust);
+			debug("Invalid FAT entry\n");
 			return -1;
 		}
+
+		cur_pos += bytesperclust;
+		curclust = newclust;
+	}
+	if (IS_LAST_CLUST(curclust, mydata->fatsize)) {
+		assert(pos == cur_pos);
+		goto set_clusters;
 	}
 
-	curclust = find_empty_cluster(mydata);
-	set_start_cluster(mydata, dentptr, curclust);
+	assert(pos < cur_pos);
+	cur_pos -= bytesperclust;
 
+	/* overwrite */
+	assert(IS_LAST_CLUST(curclust, mydata->fatsize) ||
+	       !CHECK_CLUST(curclust, mydata->fatsize));
+
+	while (1) {
+		/* search for allocated consecutive clusters */
+		actsize = bytesperclust;
+		endclust = curclust;
+		while (1) {
+			if (filesize <= (cur_pos + actsize))
+				break;
+
+			newclust = get_fatent(mydata, endclust);
+
+			if (IS_LAST_CLUST(newclust, mydata->fatsize))
+				break;
+			if (CHECK_CLUST(newclust, mydata->fatsize)) {
+				debug("curclust: 0x%x\n", curclust);
+				debug("Invalid FAT entry\n");
+				return -1;
+			}
+
+			actsize += bytesperclust;
+			endclust = newclust;
+		}
+
+		/* overwrite to <curclust..endclust> */
+		if (pos < cur_pos)
+			offset = 0;
+		else
+			offset = pos - cur_pos;
+		wsize = min(cur_pos + actsize, filesize) - pos;
+		if (get_set_cluster(mydata, curclust, offset,
+				    buffer, wsize, &actsize)) {
+			printf("Error get-and-setting cluster\n");
+			return -1;
+		}
+		buffer += wsize;
+		*gotsize += wsize;
+		cur_pos += offset + wsize;
+
+		if (filesize <= cur_pos)
+			break;
+
+		/* CHECK: newclust = get_fatent(mydata, endclust); */
+
+		if (IS_LAST_CLUST(newclust, mydata->fatsize))
+			/* no more clusters */
+			break;
+
+		curclust = newclust;
+	}
+
+	if (filesize <= cur_pos) {
+		/* no more write */
+		newclust = get_fatent(mydata, endclust);
+		if (!IS_LAST_CLUST(newclust, mydata->fatsize)) {
+			/* truncate the rest */
+			clear_fatent(mydata, newclust);
+
+			/* Mark end of file in FAT */
+			if (mydata->fatsize == 12)
+				newclust = 0xfff;
+			else if (mydata->fatsize == 16)
+				newclust = 0xffff;
+			else if (mydata->fatsize == 32)
+				newclust = 0xfffffff;
+			set_fatent_value(mydata, endclust, newclust);
+		}
+
+		return 0;
+	}
+
+	curclust = endclust;
+	filesize -= cur_pos;
+	assert(!(cur_pos % bytesperclust));
+
+set_clusters:
+	/* allocate and write */
+	assert(!pos);
+
+	/* Assure that curclust is valid */
+	if (!curclust) {
+		curclust = find_empty_cluster(mydata);
+		set_start_cluster(mydata, dentptr, curclust);
+	} else {
+		newclust = get_fatent(mydata, curclust);
+
+		if (IS_LAST_CLUST(newclust, mydata->fatsize)) {
+			newclust = determine_fatent(mydata, curclust);
+			set_fatent_value(mydata, curclust, newclust);
+			curclust = newclust;
+		} else {
+			debug("error: something wrong\n");
+			return -1;
+		}
+	}
+
+	/* TODO: already partially written */
 	if (check_overflow(mydata, curclust, filesize)) {
 		printf("Error: no space left: %llu\n", filesize);
 		return -1;
@@ -849,6 +1096,16 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 			goto exit;
 		}
 
+		/* A file exists */
+		if (pos == -1)
+			/* Append to the end */
+			pos = FAT2CPU32(retdent->size);
+		if (pos > retdent->size) {
+			/* No hole allowed */
+			ret = -EINVAL;
+			goto exit;
+		}
+
 		/* Update file size in a directory entry */
 		retdent->size = cpu_to_le32(pos + size);
 	} else {
@@ -869,6 +1126,12 @@ int file_fat_write_at(const char *filename, loff_t pos, void *buffer,
 			goto exit;
 		}
 
+		if (pos) {
+			/* No hole allowed */
+			ret = -EINVAL;
+			goto exit;
+		}
+
 		memset(itr->dent, 0, sizeof(*itr->dent));
 
 		/* Set short name to set alias checksum field in dir_slot */
@@ -918,10 +1181,5 @@ exit:
 int file_fat_write(const char *filename, void *buffer, loff_t offset,
 		   loff_t maxsize, loff_t *actwrite)
 {
-	if (offset != 0) {
-		printf("Error: non zero offset is currently not supported.\n");
-		return -EINVAL;
-	}
-
 	return file_fat_write_at(filename, offset, buffer, maxsize, actwrite);
 }
-- 
2.18.0

[U-Boot] [PATCH v2 12/23] cmd: fat: add offset parameter to fatwrite

[AKASHI Takahiro]

In this patch, fatwrite command is extended so as to accept an additional
parameter of file offset.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/fat.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/cmd/fat.c b/cmd/fat.c
index 03de5d11afb4..2a5f7bfc2690 100644
--- a/cmd/fat.c
+++ b/cmd/fat.c
@@ -104,6 +104,7 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
 	int ret;
 	unsigned long addr;
 	unsigned long count;
+	long offset;
 	struct blk_desc *dev_desc = NULL;
 	disk_partition_t info;
 	int dev = 0;
@@ -126,9 +127,11 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
 	}
 	addr = simple_strtoul(argv[3], NULL, 16);
 	count = (argc <= 5) ? 0 : simple_strtoul(argv[5], NULL, 16);
+	/* offset should be a hex, but "-1" is allowed */
+	offset = (argc <= 6) ? 0 : simple_strtol(argv[6], NULL, 16);
 
 	buf = map_sysmem(addr, count);
-	ret = file_fat_write(argv[4], buf, 0, count, &size);
+	ret = file_fat_write(argv[4], buf, offset, count, &size);
 	unmap_sysmem(buf);
 	if (ret < 0) {
 		printf("\n** Unable to write \"%s\" from %s %d:%d **\n",
@@ -142,9 +145,9 @@ static int do_fat_fswrite(cmd_tbl_t *cmdtp, int flag,
 }
 
 U_BOOT_CMD(
-	fatwrite,	6,	0,	do_fat_fswrite,
+	fatwrite,	7,	0,	do_fat_fswrite,
 	"write file into a dos filesystem",
-	"<interface> <dev[:part]> <addr> <filename> [<bytes>]\n"
+	"<interface> <dev[:part]> <addr> <filename> [<bytes> [<offset>]]\n"
 	"    - write file 'filename' from the address 'addr' in RAM\n"
 	"      to 'dev' on 'interface'"
 );
-- 
2.18.0

[U-Boot] [PATCH v2 13/23] fs: add mkdir interface

[AKASHI Takahiro]

"mkdir" interface is added to file operations.
This is a preparatory change as mkdir support for FAT file system
will be added in next patch.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fs.c      | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/fs.h | 10 ++++++++++
 2 files changed, 55 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index cb68e81cd309..62165d5c5701 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -105,6 +105,11 @@ static inline int fs_opendir_unsupported(const char *filename,
 	return -EACCES;
 }
 
+static inline int fs_mkdir_unsupported(const char *dirname)
+{
+	return -1;
+}
+
 struct fstype_info {
 	int fstype;
 	char *name;
@@ -142,6 +147,7 @@ struct fstype_info {
 	int (*readdir)(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
 	/* see fs_closedir() */
 	void (*closedir)(struct fs_dir_stream *dirs);
+	int (*mkdir)(const char *dirname);
 };
 
 static struct fstype_info fstypes[] = {
@@ -165,6 +171,7 @@ static struct fstype_info fstypes[] = {
 		.opendir = fat_opendir,
 		.readdir = fat_readdir,
 		.closedir = fat_closedir,
+		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 #ifdef CONFIG_FS_EXT4
@@ -185,6 +192,7 @@ static struct fstype_info fstypes[] = {
 #endif
 		.uuid = ext4fs_uuid,
 		.opendir = fs_opendir_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 #ifdef CONFIG_SANDBOX
@@ -201,6 +209,7 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_sandbox,
 		.uuid = fs_uuid_unsupported,
 		.opendir = fs_opendir_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 #ifdef CONFIG_CMD_UBIFS
@@ -217,6 +226,7 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_unsupported,
 		.uuid = fs_uuid_unsupported,
 		.opendir = fs_opendir_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 #ifdef CONFIG_FS_BTRFS
@@ -233,6 +243,7 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_unsupported,
 		.uuid = btrfs_uuid,
 		.opendir = fs_opendir_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 	{
@@ -248,6 +259,7 @@ static struct fstype_info fstypes[] = {
 		.write = fs_write_unsupported,
 		.uuid = fs_uuid_unsupported,
 		.opendir = fs_opendir_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 	},
 };
 
@@ -498,6 +510,20 @@ void fs_closedir(struct fs_dir_stream *dirs)
 }
 
 
+int fs_mkdir(const char *dirname)
+{
+	int ret;
+
+	struct fstype_info *info = fs_get_info(fs_type);
+
+	ret = info->mkdir(dirname);
+
+	fs_type = FS_TYPE_ANY;
+	fs_close();
+
+	return ret;
+}
+
 int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype)
 {
@@ -700,3 +726,22 @@ int do_fs_type(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return CMD_RET_SUCCESS;
 }
 
+int do_mkdir(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+	     int fstype)
+{
+	int ret;
+
+	if (argc != 4)
+		return CMD_RET_USAGE;
+
+	if (fs_set_blk_dev(argv[1], argv[2], fstype))
+		return 1;
+
+	ret = fs_mkdir(argv[3]);
+	if (ret) {
+		printf("** Unable to create a directory \"%s\" **\n", argv[3]);
+		return 1;
+	}
+
+	return 0;
+}
diff --git a/include/fs.h b/include/fs.h
index 163da103b472..fbaee154dd0d 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -155,6 +155,14 @@ struct fs_dirent *fs_readdir(struct fs_dir_stream *dirs);
  */
 void fs_closedir(struct fs_dir_stream *dirs);
 
+/*
+ * fs_mkdir - Create a directory
+ *
+ * @filename: Name of directory to create
+ * @return 0 on success, -1 on error conditions
+ */
+int fs_mkdir(const char *filename);
+
 /*
  * Common implementation for various filesystem commands, optionally limited
  * to a specific filesystem type via the fstype parameter.
@@ -169,6 +177,8 @@ int file_exists(const char *dev_type, const char *dev_part, const char *file,
 		int fstype);
 int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype);
+int do_mkdir(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+		int fstype);
 
 /*
  * Determine the UUID of the specified filesystem and print it. Optionally it is
-- 
2.18.0

[U-Boot] [PATCH v2 14/23] fs: fat: remember the starting cluster number of directory

[AKASHI Takahiro]

The starting cluster number of directory is needed to initialize ".."
(parent directory) entry when creating a new directory.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat.c  | 2 ++
 include/fat.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 50c2d4f3c33d..bbd4b098afb5 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -643,6 +643,7 @@ int fat_itr_root(fat_itr *itr, fsdata *fsdata)
 		return -ENXIO;
 
 	itr->fsdata = fsdata;
+	itr->start_clust = 0;
 	itr->clust = fsdata->root_cluster;
 	itr->next_clust = fsdata->root_cluster;
 	itr->dent = NULL;
@@ -678,6 +679,7 @@ void fat_itr_child(fat_itr *itr, fat_itr *parent)
 	assert(fat_itr_isdir(parent));
 
 	itr->fsdata = parent->fsdata;
+	itr->start_clust = clustnum;
 	if (clustnum > 0) {
 		itr->clust = clustnum;
 		itr->next_clust = clustnum;
diff --git a/include/fat.h b/include/fat.h
index 9b114e3cbda4..05a993bfdcfe 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -199,6 +199,7 @@ static inline u32 sect_to_clust(fsdata *fsdata, int sect)
 
 typedef struct {
 	fsdata    *fsdata;        /* filesystem parameters */
+	unsigned   start_clust;   /* first cluster */
 	unsigned   clust;         /* current cluster */
 	unsigned   next_clust;    /* next cluster if remaining == 0 */
 	int        last_cluster;  /* set once we've read last cluster */
-- 
2.18.0

[U-Boot] [PATCH v2 15/23] fs: fat: support mkdir

[AKASHI Takahiro]

In this patch, mkdir support is added to FAT file system.
A newly created directory contains only "." and ".." entries.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fat/fat_write.c | 136 +++++++++++++++++++++++++++++++++++++++++++++
 fs/fs.c            |   3 +-
 include/fat.h      |   1 +
 3 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 523abd62c9e9..fe0265649200 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1183,3 +1183,139 @@ int file_fat_write(const char *filename, void *buffer, loff_t offset,
 {
 	return file_fat_write_at(filename, offset, buffer, maxsize, actwrite);
 }
+
+int fat_mkdir(const char *new_dirname)
+{
+	dir_entry *retdent;
+	fsdata datablock = { .fatbuf = NULL, };
+	fsdata *mydata = &datablock;
+	fat_itr *itr = NULL;
+	char *dirname_copy, *parent, *dirname;
+	char l_dirname[VFAT_MAXLEN_BYTES];
+	int ret = -1;
+	loff_t actwrite;
+	unsigned int bytesperclust;
+	dir_entry *dotdent = NULL;
+
+	dirname_copy = strdup(new_dirname);
+	if (!dirname_copy)
+		goto exit;
+
+	split_filename(dirname_copy, &parent, &dirname);
+	if (!strlen(dirname)) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	if (normalize_longname(l_dirname, dirname)) {
+		printf("FAT: illegal filename (%s)\n", dirname);
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	itr = malloc_cache_aligned(sizeof(fat_itr));
+	if (!itr) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	ret = fat_itr_root(itr, &datablock);
+	if (ret)
+		goto exit;
+
+	total_sector = datablock.total_sect;
+
+	ret = fat_itr_resolve(itr, parent, TYPE_DIR);
+	if (ret) {
+		printf("%s: doesn't exist (%d)\n", parent, ret);
+		goto exit;
+	}
+
+	retdent = find_directory_entry(itr, l_dirname);
+
+	if (retdent) {
+		printf("%s: already exists\n", l_dirname);
+		ret = -EEXIST;
+		goto exit;
+	} else {
+		if (itr->is_root) {
+			/* root dir cannot have "." or ".." */
+			if (!strcmp(l_dirname, ".") ||
+			    !strcmp(l_dirname, "..")) {
+				ret = -EINVAL;
+				goto exit;
+			}
+		}
+
+		if (!itr->dent) {
+			printf("Error: allocating new dir entry\n");
+			ret = -EIO;
+			goto exit;
+		}
+
+		memset(itr->dent, 0, sizeof(*itr->dent));
+
+		/* Set short name to set alias checksum field in dir_slot */
+		set_name(itr->dent, dirname);
+		fill_dir_slot(itr, dirname);
+
+		/* Set attribute as archieve for regular file */
+		fill_dentry(itr->fsdata, itr->dent, dirname, 0, 0,
+			    ATTR_DIR | ATTR_ARCH);
+
+		retdent = itr->dent;
+	}
+
+	/* Default entries */
+	bytesperclust = mydata->clust_size * mydata->sect_size;
+	dotdent = malloc_cache_aligned(bytesperclust);
+	if (!dotdent) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+	memset(dotdent, 0, bytesperclust);
+
+	memcpy(dotdent[0].name, ".       ", 8);
+	memcpy(dotdent[0].ext, "   ", 3);
+	dotdent[0].attr = ATTR_DIR | ATTR_ARCH;
+
+	memcpy(dotdent[1].name, "..      ", 8);
+	memcpy(dotdent[1].ext, "   ", 3);
+	dotdent[1].attr = ATTR_DIR | ATTR_ARCH;
+	set_start_cluster(mydata, &dotdent[1], itr->start_clust);
+
+	ret = set_contents(mydata, retdent, 0, (__u8 *)dotdent,
+			   bytesperclust, &actwrite);
+	if (ret < 0) {
+		printf("Error: writing contents\n");
+		goto exit;
+	}
+	/* Write twice for "." */
+	set_start_cluster(mydata, &dotdent[0], START(retdent));
+	ret = set_contents(mydata, retdent, 0, (__u8 *)dotdent,
+			   bytesperclust, &actwrite);
+	if (ret < 0) {
+		printf("Error: writing contents\n");
+		goto exit;
+	}
+
+	/* Flush fat buffer */
+	ret = flush_dirty_fat_buffer(mydata);
+	if (ret) {
+		printf("Error: flush fat buffer\n");
+		goto exit;
+	}
+
+	/* Write directory table to device */
+	ret = set_cluster(mydata, itr->clust, itr->block,
+			  mydata->clust_size * mydata->sect_size);
+	if (ret)
+		printf("Error: writing directory entry\n");
+
+exit:
+	free(dirname_copy);
+	free(mydata->fatbuf);
+	free(itr);
+	free(dotdent);
+	return ret;
+}
diff --git a/fs/fs.c b/fs/fs.c
index 62165d5c5701..099540f38a10 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -164,14 +164,15 @@ static struct fstype_info fstypes[] = {
 		.read = fat_read_file,
 #ifdef CONFIG_FAT_WRITE
 		.write = file_fat_write,
+		.mkdir = fat_mkdir,
 #else
 		.write = fs_write_unsupported,
+		.mkdir = fs_mkdir_unsupported,
 #endif
 		.uuid = fs_uuid_unsupported,
 		.opendir = fat_opendir,
 		.readdir = fat_readdir,
 		.closedir = fat_closedir,
-		.mkdir = fs_mkdir_unsupported,
 	},
 #endif
 #ifdef CONFIG_FS_EXT4
diff --git a/include/fat.h b/include/fat.h
index 05a993bfdcfe..c34eb8cec6bd 100644
--- a/include/fat.h
+++ b/include/fat.h
@@ -239,6 +239,7 @@ int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len,
 int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
 int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
 void fat_closedir(struct fs_dir_stream *dirs);
+int fat_mkdir(const char *dirname);
 void fat_close(void);
 #endif /* CONFIG_FS_FAT */
 #endif /* _FAT_H_ */
-- 
2.18.0

[U-Boot] [PATCH v2 16/23] cmd: fat: add fatmkdir command

[AKASHI Takahiro]

In this patch, a new command, fatmkdir, is added.

Please note that, as there is no notion of "current directory" on u-boot,
a directory name specified must contains an absolute directory path as
a parent directory. Otherwise, "/" (root directory) is assumed.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/fat.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/cmd/fat.c b/cmd/fat.c
index 2a5f7bfc2690..b685bf70a2b3 100644
--- a/cmd/fat.c
+++ b/cmd/fat.c
@@ -151,4 +151,17 @@ U_BOOT_CMD(
 	"    - write file 'filename' from the address 'addr' in RAM\n"
 	"      to 'dev' on 'interface'"
 );
+
+static int do_fat_mkdir(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	return do_mkdir(cmdtp, flag, argc, argv, FS_TYPE_FAT);
+}
+
+U_BOOT_CMD(
+	fatmkdir,	4,	1,	do_fat_mkdir,
+	"create a directory",
+	"<interface> [<dev[:part]>] <directory>\n"
+	"    - create a directory in 'dev' on 'interface'"
+);
 #endif
-- 
2.18.0

[U-Boot] [PATCH v2 17/23] efi_loader: file: support creating a directory

[AKASHI Takahiro]

In efi world, there is no obvious "mkdir" interface, instead, Open()
with EFI_FILE_MODE_CREATE in mode parameter and EFI_FILE_DIRECTORY
in attributes parameter creates a directory.

In this patch, efi_file_open() is extended so as to accept such
a combination of parameters and call u-boot's mkdir interface for
expected action.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/efi_file.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index e6a15bcb523e..6ec98c80227e 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -130,7 +130,8 @@ static int sanitize_path(char *path)
  * With windoze style backlashes, ofc.
  */
 static struct efi_file_handle *file_open(struct file_system *fs,
-		struct file_handle *parent, s16 *file_name, u64 mode)
+		struct file_handle *parent, s16 *file_name, u64 mode,
+		u64 attributes)
 {
 	struct file_handle *fh;
 	char f0[MAX_UTF8_PER_UTF16] = {0};
@@ -173,7 +174,12 @@ static struct efi_file_handle *file_open(struct file_system *fs,
 		if (set_blk_dev(fh))
 			goto error;
 
-		if (!((mode & EFI_FILE_MODE_CREATE) || fs_exists(fh->path)))
+		if ((mode & EFI_FILE_MODE_CREATE) &&
+		    (attributes & EFI_FILE_DIRECTORY)) {
+			if (fs_mkdir(fh->path))
+				goto error;
+		} else if (!((mode & EFI_FILE_MODE_CREATE) ||
+			     fs_exists(fh->path)))
 			goto error;
 
 		/* figure out if file is a directory: */
@@ -199,7 +205,7 @@ static efi_status_t EFIAPI efi_file_open(struct efi_file_handle *file,
 	EFI_ENTRY("%p, %p, \"%ls\", %llx, %llu", file, new_handle, file_name,
 		  open_mode, attributes);
 
-	*new_handle = file_open(fh->fs, fh, file_name, open_mode);
+	*new_handle = file_open(fh->fs, fh, file_name, open_mode, attributes);
 	if (!*new_handle)
 		return EFI_EXIT(EFI_NOT_FOUND);
 
@@ -598,7 +604,7 @@ efi_open_volume(struct efi_simple_file_system_protocol *this,
 
 	EFI_ENTRY("%p, %p", this, root);
 
-	*root = file_open(fs, NULL, NULL, 0);
+	*root = file_open(fs, NULL, NULL, 0, 0);
 
 	return EFI_EXIT(EFI_SUCCESS);
 }
-- 
2.18.0

[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[AKASHI Takahiro]

This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
Returning EFI_SUCCESS can cheat SCT execution.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/efi_file.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index 6ec98c80227e..12044a0c7196 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -226,12 +226,20 @@ static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file)
 	return EFI_EXIT(file_close(fh));
 }
 
+static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file,
+					  efi_uintn_t *buffer_size,
+					  void *buffer);
+
 static efi_status_t EFIAPI efi_file_delete(struct efi_file_handle *file)
 {
 	struct file_handle *fh = to_fh(file);
+	efi_uintn_t size = 0;
 	EFI_ENTRY("%p", file);
+
+	/* TODO: implement real 'delete' */
+	efi_file_write(file, &size, NULL);
 	file_close(fh);
-	return EFI_EXIT(EFI_WARN_DELETE_FAILURE);
+	return EFI_SUCCESS;
 }
 
 static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
-- 
2.18.0

[U-Boot] [PATCH v2 19/23] fs-test: fix false positive error at Test Case 12

[AKASHI Takahiro]

The error message to be matched is wrong. Fix it.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/fs/fs-test.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/fs/fs-test.sh b/test/fs/fs-test.sh
index 9482239562ea..e002b9105131 100755
--- a/test/fs/fs-test.sh
+++ b/test/fs/fs-test.sh
@@ -522,7 +522,7 @@ function check_results() {
 		"TC11: 1MB write to $3.w - content verified"
 
 	# Check lookup of 'dot' directory
-	grep -A4 "Test Case 12 " "$1" | grep -q 'Unable to write file'
+	grep -A4 "Test Case 12 " "$1" | grep -q 'Unable to write'
 	pass_fail "TC12: 1MB write to . - write denied"
 
 	# Check directory traversal
-- 
2.18.0

[U-Boot] [PATCH v2 20/23] fs-test: update the test result as of v2018.09

[AKASHI Takahiro]

As far as this patch series has been applied, all the tests should
pass. So update the test result summary.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/fs/fs-test.sh | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/test/fs/fs-test.sh b/test/fs/fs-test.sh
index e002b9105131..86308cfe2dbd 100755
--- a/test/fs/fs-test.sh
+++ b/test/fs/fs-test.sh
@@ -7,18 +7,20 @@
 # It currently tests the fs/sb and native commands for ext4 and fat partitions
 # Expected results are as follows:
 # EXT4 tests:
-# fs-test.sb.ext4.out: Summary: PASS: 24 FAIL: 0
-# fs-test.ext4.out: Summary: PASS: 24 FAIL: 0
-# fs-test.fs.ext4.out: Summary: PASS: 24 FAIL: 0
+# fs-test.sb.ext4	Summary: PASS: 24 FAIL: 0
+# fs-test.nonfs.ext4	Summary: PASS: 24 FAIL: 0
+# fs-test.fs.ext4	Summary: PASS: 24 FAIL: 0
 # FAT16 tests:
-# fs-test.sb.fat16.out: Summary: PASS: 24 FAIL: 0
-# fs-test.fat16.out: Summary: PASS: 20 FAIL: 4
-# fs-test.fs.fat16.out: Summary: PASS: 20 FAIL: 4
+# fs-test.sb.fat16	Summary: PASS: 24 FAIL: 0
+# fs-test.nonfs.fat16	Summary: PASS: 24 FAIL: 0
+# fs-test.fs.fat16	Summary: PASS: 24 FAIL: 0
 # FAT32 tests:
-# fs-test.sb.fat32.out: Summary: PASS: 24 FAIL: 0
-# fs-test.fat32.out: Summary: PASS: 20 FAIL: 4
-# fs-test.fs.fat32.out: Summary: PASS: 20 FAIL: 4
-# Total Summary: TOTAL PASS: 200 TOTAL FAIL: 16
+# fs-test.sb.fat32	Summary: PASS: 24 FAIL: 0
+# fs-test.nonfs.fat32	Summary: PASS: 24 FAIL: 0
+# fs-test.fs.fat32	Summary: PASS: 24 FAIL: 0
+# --------------------------------------------
+# Total Summary: TOTAL PASS: 216 TOTAL FAIL: 0
+# --------------------------------------------
 
 # pre-requisite binaries list.
 PREREQ_BINS="md5sum mkfs mount umount dd fallocate mkdir"
-- 
2.18.0

[U-Boot] [PATCH v2 21/23] test/py: convert fs-test.sh to pytest

[AKASHI Takahiro]

In this commit, the same set of test cases as in test/fs/fs-test.sh
is provided using pytest framework.
Actually, fs-test.sh provides three variants:"sb" (sb command), "nonfs"
(fatxx and etc.) and "fs" (hostfs), and this patch currently supports
only "nonfs" variant; So it is not a replacement of fs-test.sh for now.

Simple usage:
  $ py.test test/py/tests/test_fs [<other options>]

You may also specify filesystem types to be tested:
  $ py.test test/py/tests/test_fs --fs-type fat32 [<other options>]

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/tests/test_fs/conftest.py    | 216 ++++++++++++++++++++
 test/py/tests/test_fs/fstest_defs.py |  10 +
 test/py/tests/test_fs/test_basic.py  | 287 +++++++++++++++++++++++++++
 3 files changed, 513 insertions(+)
 create mode 100644 test/py/tests/test_fs/conftest.py
 create mode 100644 test/py/tests/test_fs/fstest_defs.py
 create mode 100644 test/py/tests/test_fs/test_basic.py

diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
new file mode 100644
index 000000000000..9155ccf84266
--- /dev/null
+++ b/test/py/tests/test_fs/conftest.py
@@ -0,0 +1,216 @@
+# SPDX-License-Identifier:      GPL-2.0+
+# Copyright (c) 2018, Linaro Limited
+# Author: Takahiro Akashi <takahiro.akashi@linaro.org>
+
+import os
+import os.path
+import pytest
+import re
+from subprocess import call, check_call, check_output, CalledProcessError
+from fstest_defs import *
+
+supported_fs_basic = ['fat16', 'fat32', 'ext4']
+
+#
+# Filesystem test specific setup
+#
+def pytest_addoption(parser):
+    parser.addoption('--fs-type', action='append', default=None,
+        help='Targeting Filesystem Types')
+
+def pytest_configure(config):
+    global supported_fs_basic
+
+    def intersect(listA, listB):
+        return  [x for x in listA if x in listB]
+
+    supported_fs = config.getoption('fs_type')
+    if supported_fs:
+        print("*** FS TYPE modified: %s" % supported_fs)
+        supported_fs_basic =  intersect(supported_fs, supported_fs_basic)
+
+def pytest_generate_tests(metafunc):
+    if 'fs_obj_basic' in metafunc.fixturenames:
+        metafunc.parametrize('fs_obj_basic', supported_fs_basic,
+            indirect=True, scope='module')
+
+#
+# Helper functions
+#
+def fstype_to_ubname(fs_type):
+    if re.match('fat', fs_type):
+        return 'fat'
+    else:
+        return fs_type
+
+def check_ubconfig(config, fs_type):
+    if not config.buildconfig.get('config_cmd_%s' % fs_type, None):
+        pytest.skip('.config feature "CMD_%s" not enabled' % fs_type.upper())
+    if not config.buildconfig.get('config_%s_write' % fs_type, None):
+        pytest.skip('.config feature "%s_WRITE" not enabled'
+        % fs_type.upper())
+
+def mk_fs(config, fs_type, size, id):
+    fs_img = '%s.%s.img' % (id, fs_type)
+    fs_img = config.persistent_data_dir + '/' + fs_img
+
+    if fs_type == 'fat16':
+        mkfs_opt = '-F 16'
+    elif fs_type == 'fat32':
+        mkfs_opt = '-F 32'
+    else:
+        mkfs_opt = ''
+
+    if re.match('fat', fs_type):
+        fs_lnxtype = 'vfat'
+    else:
+        fs_lnxtype = fs_type
+
+    count = (size + 1048576 - 1) / 1048576
+
+    try:
+        check_call('rm -f %s' % fs_img, shell=True)
+        check_call('dd if=/dev/zero of=%s bs=1M count=%d'
+            % (fs_img, count), shell=True)
+        check_call('mkfs.%s %s %s'
+            % (fs_lnxtype, mkfs_opt, fs_img), shell=True)
+        return fs_img
+    except CalledProcessError:
+        call('rm -f %s' % fs_img, shell=True)
+        raise
+
+# from test/py/conftest.py
+def tool_is_in_path(tool):
+    for path in os.environ["PATH"].split(os.pathsep):
+        fn = os.path.join(path, tool)
+        if os.path.isfile(fn) and os.access(fn, os.X_OK):
+            return True
+    return False
+
+fuse_mounted = False
+
+def mount_fs(config, fs_type, device, mount_point):
+    fuse_mounted = False
+
+    try:
+        if fs_type == 'ext4' and tool_is_in_path('fuse2fs'):
+            check_call('fuse2fs %s %s' % (device, mount_point), shell=True)
+            fuse_mounted = True
+        else:
+            mount_opt = "loop,rw"
+            if re.match('fat', fs_type):
+                mount_opt += ",umask=0000"
+
+            check_call('sudo mount -o %s %s %s'
+                % (mount_opt, device, mount_point), shell=True)
+
+        # may not be effective for some file systems
+        check_call('sudo chmod a+rw %s' % mount_point, shell=True)
+    except CalledProcessError:
+        raise
+
+def umount_fs(config, fs_type, mount_point):
+    if fuse_mounted:
+        call('fusermount -u %s' % mount_point, shell=True)
+    else:
+        call('sudo umount %s' % mount_point, shell=True)
+
+
+#
+# Fixture for basic fs test
+#     derived from test/fs/fs-test.sh
+#
+# NOTE: yield_fixture was deprecated since pytest-3.0
+ at pytest.yield_fixture()
+def fs_obj_basic(request, u_boot_config):
+    fs_type = request.param
+    fs_img = ''
+
+    fs_ubtype = fstype_to_ubname(fs_type)
+    check_ubconfig(u_boot_config, fs_ubtype)
+
+    mount_dir = u_boot_config.persistent_data_dir + '/mnt'
+
+    small_file = mount_dir + '/' + SMALL_FILE
+    big_file = mount_dir + '/' + BIG_FILE
+
+    try:
+
+        # 3GiB volume
+        fs_img = mk_fs(u_boot_config, fs_type, 0xc0000000, '3GB')
+
+        # Mount the image so we can populate it.
+        check_call('mkdir -p %s' % mount_dir, shell=True)
+        mount_fs(u_boot_config, fs_type, fs_img, mount_dir)
+
+        # Create a subdirectory.
+        check_call('mkdir %s/SUBDIR' % mount_dir, shell=True)
+
+        # Create big file in this image.
+        # Note that we work only on the start 1MB, couple MBs in the 2GB range
+        # and the last 1 MB of the huge 2.5GB file.
+        # So, just put random values only in those areas.
+        check_call('dd if=/dev/urandom of=%s bs=1M count=1'
+	    % big_file, shell=True)
+        check_call('dd if=/dev/urandom of=%s bs=1M count=2 seek=2047'
+            % big_file, shell=True)
+        check_call('dd if=/dev/urandom of=%s bs=1M count=1 seek=2499'
+            % big_file, shell=True)
+
+        # Create a small file in this image.
+        check_call('dd if=/dev/urandom of=%s bs=1M count=1'
+	    % small_file, shell=True)
+
+        # Delete the small file copies which possibly are written as part of a
+        # previous test.
+        # check_call('rm -f "%s.w"' % MB1, shell=True)
+        # check_call('rm -f "%s.w2"' % MB1, shell=True)
+
+        # Generate the md5sums of reads that we will test against small file
+        out = check_output(
+            'dd if=%s bs=1M skip=0 count=1 2> /dev/null | md5sum'
+	    % small_file, shell=True)
+        md5val = [ out.split()[0] ]
+
+        # Generate the md5sums of reads that we will test against big file
+        # One from beginning of file.
+        out = check_output(
+            'dd if=%s bs=1M skip=0 count=1 2> /dev/null | md5sum'
+	    % big_file, shell=True)
+        md5val.append(out.split()[0])
+
+        # One from end of file.
+        out = check_output(
+            'dd if=%s bs=1M skip=2499 count=1 2> /dev/null | md5sum'
+	    % big_file, shell=True)
+        md5val.append(out.split()[0])
+
+        # One from the last 1MB chunk of 2GB
+        out = check_output(
+            'dd if=%s bs=1M skip=2047 count=1 2> /dev/null | md5sum'
+	    % big_file, shell=True)
+        md5val.append(out.split()[0])
+
+        # One from the start 1MB chunk from 2GB
+        out = check_output(
+            'dd if=%s bs=1M skip=2048 count=1 2> /dev/null | md5sum'
+	    % big_file, shell=True)
+        md5val.append(out.split()[0])
+
+        # One 1MB chunk crossing the 2GB boundary
+        out = check_output(
+            'dd if=%s bs=512K skip=4095 count=2 2> /dev/null | md5sum'
+	    % big_file, shell=True)
+        md5val.append(out.split()[0])
+
+        umount_fs(u_boot_config, fs_type, mount_dir)
+    except CalledProcessError:
+        pytest.skip('Setup failed for filesystem: ' + fs_type)
+        return
+    else:
+        yield [fs_ubtype, fs_img, md5val]
+    finally:
+        umount_fs(u_boot_config, fs_type, mount_dir)
+        call('rmdir %s' % mount_dir, shell=True)
+#        if fs_img:
+#            call('rm -f %s' % fs_img, shell=True)
diff --git a/test/py/tests/test_fs/fstest_defs.py b/test/py/tests/test_fs/fstest_defs.py
new file mode 100644
index 000000000000..f26dd06cacf2
--- /dev/null
+++ b/test/py/tests/test_fs/fstest_defs.py
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier:      GPL-2.0+
+
+# $SMALL_FILE is the name of the 1MB file in the file system image
+SMALL_FILE='1MB.file'
+
+# $BIG_FILE is the name of the 2.5GB file in the file system image
+BIG_FILE='2.5GB.file'
+
+ADDR=0x01000008
+LENGTH=0x00100000
diff --git a/test/py/tests/test_fs/test_basic.py b/test/py/tests/test_fs/test_basic.py
new file mode 100644
index 000000000000..c067cc9ba3f6
--- /dev/null
+++ b/test/py/tests/test_fs/test_basic.py
@@ -0,0 +1,287 @@
+# SPDX-License-Identifier:      GPL-2.0+
+# Copyright (c) 2018, Linaro Limited
+# Author: Takahiro Akashi <takahiro.akashi@linaro.org>
+#
+# U-Boot File System:Basic Test
+
+"""
+This test verifies basic read/write operation on file system.
+"""
+
+import pytest
+import re
+from fstest_defs import *
+
+ at pytest.mark.boardspec('sandbox')
+class TestFsBasic(object):
+    def test_fs1(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 1 - ls command, listing a root directory and invalid directory
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 1a - ls'):
+            # Test Case 1 - ls
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sls host 0:0' % fs_type])
+            assert(re.search('2621440000 *%s' % BIG_FILE, ''.join(output)))
+            assert(re.search('1048576 *%s' % SMALL_FILE, ''.join(output)))
+
+        with u_boot_console.log.section('Test Case 1b - ls (invalid dir)'):
+            # In addition, test with a nonexistent directory to see if we crash.
+            output = u_boot_console.run_command(
+                '%sls host 0:0 invalid_d' % fs_type)
+            if fs_type == 'ext4':
+                assert('Can not find directory' in output)
+            else:
+                assert('' == output)
+
+    def test_fs2(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 2 - size command for a small file
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 2a - size (small)'):
+            # 1MB is 0x0010 0000
+            # Test Case 2a - size of small file
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%ssize host 0:0 /%s' % (fs_type, SMALL_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+        with u_boot_console.log.section('Test Case 2b - size (/../<file>)'):
+            # Test Case 2b - size of small file via a path using '..'
+            output = u_boot_console.run_command_list([
+                '%ssize host 0:0 /SUBDIR/../%s' % (fs_type, SMALL_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+    def test_fs3(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 3 - size command for a large file
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 3 - size (large)'):
+            # 2.5GB (1024*1024*2500) is 0x9C40 0000
+            # Test Case 3 - size of big file
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%ssize host 0:0 /%s' % (fs_type, BIG_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=9c400000' in ''.join(output))
+
+    def test_fs4(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 4 - load a small file, 1MB
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 4 - load (small)'):
+            # Test Case 4a - Read full 1MB of small file
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, SMALL_FILE),
+                'printenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+            # Test Case 4b - Read full 1MB of small file
+            output = u_boot_console.run_command_list([
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[0] in ''.join(output))
+
+    def test_fs5(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 5 - load, reading first 1MB of 3GB file
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 5 - load (first 1MB)'):
+            # Test Case 5a - First 1MB of big file
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s %x 0x0' % (fs_type, ADDR, BIG_FILE, LENGTH),
+                'printenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+            # Test Case 5b - First 1MB of big file
+            output = u_boot_console.run_command_list([
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[1] in ''.join(output))
+
+    def test_fs6(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 6 - load, reading last 1MB of 3GB file
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 6 - load (last 1MB)'):
+            # fails for ext as no offset support
+            # Test Case 6a - Last 1MB of big file
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s %x 0x9c300000'
+                    % (fs_type, ADDR, BIG_FILE, LENGTH),
+                'printenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+            # Test Case 6b - Last 1MB of big file
+            output = u_boot_console.run_command_list([
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[2] in ''.join(output))
+
+    def test_fs7(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 7 - load, 1MB from the last 1MB in 2GB
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 7 - load (last 1MB in 2GB)'):
+            # fails for ext as no offset support
+            # Test Case 7a - One from the last 1MB chunk of 2GB
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s %x 0x7ff00000'
+                    % (fs_type, ADDR, BIG_FILE, LENGTH),
+                'printenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+            # Test Case 7b - One from the last 1MB chunk of 2GB
+            output = u_boot_console.run_command_list([
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[3] in ''.join(output))
+
+    def test_fs8(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 8 - load, reading first 1MB in 2GB
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 8 - load (first 1MB in 2GB)'):
+            # fails for ext as no offset support
+            # Test Case 8a - One from the start 1MB chunk from 2GB
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s %x 0x80000000'
+                    % (fs_type, ADDR, BIG_FILE, LENGTH),
+                'printenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+            # Test Case 8b - One from the start 1MB chunk from 2GB
+            output = u_boot_console.run_command_list([
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[4] in ''.join(output))
+
+    def test_fs9(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 9 - load, 1MB crossing 2GB boundary
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 9 - load (crossing 2GB boundary)'):
+            # fails for ext as no offset support
+            # Test Case 9a - One 1MB chunk crossing the 2GB boundary
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s %x 0x7ff80000'
+                    % (fs_type, ADDR, BIG_FILE, LENGTH),
+                'printenv filesize'])
+            assert('filesize=100000' in ''.join(output))
+
+            # Test Case 9b - One 1MB chunk crossing the 2GB boundary
+            output = u_boot_console.run_command_list([
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[5] in ''.join(output))
+
+    def test_fs10(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 10 - load, reading beyond file end'):
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 10 - load (beyond file end)'):
+            # Generic failure case
+            # Test Case 10 - 2MB chunk from the last 1MB of big file
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s 0x00200000 0x9c300000'
+                    % (fs_type, ADDR, BIG_FILE),
+                'printenv filesize',
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+        assert('filesize=100000' in ''.join(output))
+
+    def test_fs11(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 11 - write'
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 11 - write'):
+            # Read 1MB from small file
+            # Write it back to test the writes
+            # Test Case 11a - Check that the write succeeded
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, SMALL_FILE),
+                '%swrite host 0:0 %x /%s.w $filesize'
+                    % (fs_type, ADDR, SMALL_FILE)])
+            assert('1048576 bytes written' in ''.join(output))
+
+            # Test Case 11b - Check md5 of written to is same
+            # as the one read from
+            output = u_boot_console.run_command_list([
+                '%sload host 0:0 %x /%s.w' % (fs_type, ADDR, SMALL_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[0] in ''.join(output))
+
+    def test_fs12(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 12 - write to "." directory
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 12 - write (".")'):
+            # Next test case checks writing a file whose dirent
+            # is the first in the block, which is always true for "."
+            # The write should fail, but the lookup should work
+            # Test Case 12 - Check directory traversal
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%swrite host 0:0 %x /. 0x10' % (fs_type, ADDR)])
+            assert('Unable to write' in ''.join(output))
+
+    def test_fs13(self, u_boot_console, fs_obj_basic):
+        """
+        Test Case 13 - write to a file with "/./<filename>"
+        """
+        fs_type,fs_img,md5val = fs_obj_basic
+        with u_boot_console.log.section('Test Case 13 - write  ("./<file>")'):
+            # Read 1MB from small file
+            # Write it via "same directory", i.e. "." dirent
+            # Test Case 13a - Check directory traversal
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, SMALL_FILE),
+                '%swrite host 0:0 %x /./%s2 $filesize'
+                    % (fs_type, ADDR, SMALL_FILE)])
+            assert('1048576 bytes written' in ''.join(output))
+
+            # Test Case 13b - Check md5 of written to is same
+            # as the one read from
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x /./%s2' % (fs_type, ADDR, SMALL_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[0] in ''.join(output))
+
+            # Test Case 13c - Check md5 of written to is same
+            # as the one read from
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x /%s2' % (fs_type, ADDR, SMALL_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[0] in ''.join(output))
-- 
2.18.0

[U-Boot] [PATCH v2 22/23] test/py: fs: add extended write operation test

[AKASHI Takahiro]

In this commit and the next one, test scripts for new filesystem
functionalities introduced by my patch set, "fs: fat: extend FAT write
operations," are provided.

In particular, this patch adds test cases for sub-directory write
and write with non-zero offset.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/tests/test_fs/conftest.py    |  84 ++++++++++
 test/py/tests/test_fs/fstest_defs.py |   3 +
 test/py/tests/test_fs/test_ext.py    | 224 +++++++++++++++++++++++++++
 3 files changed, 311 insertions(+)
 create mode 100644 test/py/tests/test_fs/test_ext.py

diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
index 9155ccf84266..f72d53d88fc8 100644
--- a/test/py/tests/test_fs/conftest.py
+++ b/test/py/tests/test_fs/conftest.py
@@ -10,6 +10,7 @@ from subprocess import call, check_call, check_output, CalledProcessError
 from fstest_defs import *
 
 supported_fs_basic = ['fat16', 'fat32', 'ext4']
+supported_fs_ext = ['fat16', 'fat32']
 
 #
 # Filesystem test specific setup
@@ -20,6 +21,7 @@ def pytest_addoption(parser):
 
 def pytest_configure(config):
     global supported_fs_basic
+    global supported_fs_ext
 
     def intersect(listA, listB):
         return  [x for x in listA if x in listB]
@@ -28,11 +30,15 @@ def pytest_configure(config):
     if supported_fs:
         print("*** FS TYPE modified: %s" % supported_fs)
         supported_fs_basic =  intersect(supported_fs, supported_fs_basic)
+        supported_fs_ext =  intersect(supported_fs, supported_fs_ext)
 
 def pytest_generate_tests(metafunc):
     if 'fs_obj_basic' in metafunc.fixturenames:
         metafunc.parametrize('fs_obj_basic', supported_fs_basic,
             indirect=True, scope='module')
+    if 'fs_obj_ext' in metafunc.fixturenames:
+        metafunc.parametrize('fs_obj_ext', supported_fs_ext,
+            indirect=True, scope='module')
 
 #
 # Helper functions
@@ -214,3 +220,81 @@ def fs_obj_basic(request, u_boot_config):
         call('rmdir %s' % mount_dir, shell=True)
 #        if fs_img:
 #            call('rm -f %s' % fs_img, shell=True)
+
+
+#
+# Fixture for extended fs test
+#
+# NOTE: yield_fixture was deprecated since pytest-3.0
+ at pytest.yield_fixture()
+def fs_obj_ext(request, u_boot_config):
+    fs_type = request.param
+    fs_img = ''
+
+    fs_ubtype = fstype_to_ubname(fs_type)
+    check_ubconfig(u_boot_config, fs_ubtype)
+
+    mount_dir = u_boot_config.persistent_data_dir + '/mnt'
+
+    min_file = mount_dir + '/' + MIN_FILE
+    tmp_file = mount_dir + '/tmpfile'
+
+    try:
+
+        # 128MiB volume
+        fs_img = mk_fs(u_boot_config, fs_type, 0x8000000, '128MB')
+
+        # Mount the image so we can populate it.
+        check_call('mkdir -p %s' % mount_dir, shell=True)
+        mount_fs(u_boot_config, fs_type, fs_img, mount_dir)
+
+        # Create a test directory
+        check_call('mkdir %s/dir1' % mount_dir, shell=True)
+
+        # Create a small file and calculate md5
+        check_call('dd if=/dev/urandom of=%s bs=1K count=20'
+            % min_file, shell=True)
+        out = check_output(
+            'dd if=%s bs=1K 2> /dev/null | md5sum'
+            % min_file, shell=True)
+        md5val = [ out.split()[0] ]
+
+        # Calculate md5sum of Test Case 4
+        check_call('dd if=%s of=%s bs=1K count=20'
+            % (min_file, tmp_file), shell=True)
+        check_call('dd if=%s of=%s bs=1K seek=5 count=20'
+            % (min_file, tmp_file), shell=True)
+        out = check_output('dd if=%s bs=1K 2> /dev/null | md5sum'
+            % tmp_file, shell=True)
+        md5val.append(out.split()[0])
+
+        # Calculate md5sum of Test Case 5
+        check_call('dd if=%s of=%s bs=1K count=20'
+            % (min_file, tmp_file), shell=True)
+        check_call('dd if=%s of=%s bs=1K seek=5 count=5'
+            % (min_file, tmp_file), shell=True)
+        out = check_output('dd if=%s bs=1K 2> /dev/null | md5sum'
+            % tmp_file, shell=True)
+        md5val.append(out.split()[0])
+
+        # Calculate md5sum of Test Case 7
+        check_call('dd if=%s of=%s bs=1K count=20'
+            % (min_file, tmp_file), shell=True)
+        check_call('dd if=%s of=%s bs=1K seek=20 count=20'
+            % (min_file, tmp_file), shell=True)
+        out = check_output('dd if=%s bs=1K 2> /dev/null | md5sum'
+            % tmp_file, shell=True)
+        md5val.append(out.split()[0])
+
+        check_call('rm %s' % tmp_file, shell=True)
+        umount_fs(u_boot_config, fs_type, mount_dir)
+    except CalledProcessError:
+        pytest.skip('Setup failed for filesystem: ' + fs_type)
+        return
+    else:
+        yield [fs_ubtype, fs_img, md5val]
+    finally:
+        umount_fs(u_boot_config, fs_type, mount_dir)
+        call('rmdir %s' % mount_dir, shell=True)
+#        if fs_img:
+#            call('rm -f %s' % fs_img, shell=True)
diff --git a/test/py/tests/test_fs/fstest_defs.py b/test/py/tests/test_fs/fstest_defs.py
index f26dd06cacf2..5f107562d952 100644
--- a/test/py/tests/test_fs/fstest_defs.py
+++ b/test/py/tests/test_fs/fstest_defs.py
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier:      GPL-2.0+
 
+# $MIN_FILE is the name of the 20KB file in the file system image
+MIN_FILE='testfile'
+
 # $SMALL_FILE is the name of the 1MB file in the file system image
 SMALL_FILE='1MB.file'
 
diff --git a/test/py/tests/test_fs/test_ext.py b/test/py/tests/test_fs/test_ext.py
new file mode 100644
index 000000000000..38217d08bf64
--- /dev/null
+++ b/test/py/tests/test_fs/test_ext.py
@@ -0,0 +1,224 @@
+# SPDX-License-Identifier:      GPL-2.0+
+# Copyright (c) 2018, Linaro Limited
+# Author: Takahiro Akashi <takahiro.akashi@linaro.org>
+#
+# U-Boot File System:Exntented Test
+
+"""
+This test verifies extended write operation on file system.
+"""
+
+import pytest
+import re
+from fstest_defs import *
+
+ at pytest.mark.boardspec('sandbox')
+class TestFsExt(object):
+    def test_fs_ext1(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 1 - write a file with absolute path
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 1 - write with abs path'):
+            # Test Case 1a - Check if command successfully returned
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w1 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            assert('20480 bytes written' in ''.join(output))
+
+            # Test Case 1b - Check md5 of file content
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x /dir1/%s.w1' % (fs_type, ADDR, MIN_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[0] in ''.join(output))
+
+    def test_fs_ext2(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 2 - write to a file with relative path
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 2 - write with rel path'):
+            # Test Case 2a - Check if command successfully returned
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x dir1/%s.w2 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            assert('20480 bytes written' in ''.join(output))
+
+            # Test Case 2b - Check md5 of file content
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x dir1/%s.w2' % (fs_type, ADDR, MIN_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[0] in ''.join(output))
+
+    def test_fs_ext3(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 3 - write to a file with invalid path
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 3 - write with invalid path'):
+            # Test Case 3 - Check if command expectedly failed
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/none/%s.w3 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            assert('Unable to write "/dir1/none/' in ''.join(output))
+
+    def test_fs_ext4(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 4 - write at non-zero offset, enlarging file size
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 4 - write at non-zero offset, enlarging file size'):
+            # Test Case 4a - Check if command successfully returned
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w4 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            output = u_boot_console.run_command(
+                '%swrite host 0:0 %x /dir1/%s.w4 $filesize 0x1400'
+                    % (fs_type, ADDR, MIN_FILE))
+            assert('20480 bytes written' in output)
+
+            # Test Case 4b - Check size of written file
+            output = u_boot_console.run_command_list([
+                '%ssize host 0:0 /dir1/%s.w4' % (fs_type, MIN_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=6400' in ''.join(output))
+
+            # Test Case 4c - Check md5 of file content
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x /dir1/%s.w4' % (fs_type, ADDR, MIN_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[1] in ''.join(output))
+
+    def test_fs_ext5(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 5 - write at non-zero offset, shrinking file size
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 5 - write at non-zero offset, shrinking file size'):
+            # Test Case 5a - Check if command successfully returned
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w5 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            output = u_boot_console.run_command(
+                '%swrite host 0:0 %x /dir1/%s.w5 0x1400 0x1400'
+                    % (fs_type, ADDR, MIN_FILE))
+            assert('5120 bytes written' in output)
+
+            # Test Case 5b - Check size of written file
+            output = u_boot_console.run_command_list([
+                '%ssize host 0:0 /dir1/%s.w5' % (fs_type, MIN_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=2800' in ''.join(output))
+
+            # Test Case 5c - Check md5 of file content
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x /dir1/%s.w5' % (fs_type, ADDR, MIN_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[2] in ''.join(output))
+
+    def test_fs_ext6(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 6 - write nothing at the start, truncating to zero
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 6 - write nothing at the start, truncating to zero'):
+            # Test Case 6a - Check if command successfully returned
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w6 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            output = u_boot_console.run_command(
+                '%swrite host 0:0 %x /dir1/%s.w6 0 0'
+                    % (fs_type, ADDR, MIN_FILE))
+            assert('0 bytes written' in output)
+
+            # Test Case 6b - Check size of written file
+            output = u_boot_console.run_command_list([
+                '%ssize host 0:0 /dir1/%s.w6' % (fs_type, MIN_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=0' in ''.join(output))
+
+    def test_fs_ext7(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 7 - write at the end (append)
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 7 - write at the end (append)'):
+            # Test Case 7a - Check if command successfully returned
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w7 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            output = u_boot_console.run_command(
+                '%swrite host 0:0 %x /dir1/%s.w7 $filesize $filesize'
+                    % (fs_type, ADDR, MIN_FILE))
+            assert('20480 bytes written' in output)
+
+            # Test Case 7b - Check size of written file
+            output = u_boot_console.run_command_list([
+                '%ssize host 0:0 /dir1/%s.w7' % (fs_type, MIN_FILE),
+                'printenv filesize',
+                'setenv filesize'])
+            assert('filesize=a000' in ''.join(output))
+
+            # Test Case 7c - Check md5 of file content
+            output = u_boot_console.run_command_list([
+                'mw.b %x 00 100' % ADDR,
+                '%sload host 0:0 %x /dir1/%s.w7' % (fs_type, ADDR, MIN_FILE),
+                'md5sum %x $filesize' % ADDR,
+                'setenv filesize'])
+            assert(md5val[3] in ''.join(output))
+
+    def test_fs_ext8(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 8 - write at offset beyond the end of file
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 8 - write beyond the end'):
+            # Test Case 8a - Check if command expectedly failed
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w8 $filesize'
+                    % (fs_type, ADDR, MIN_FILE)])
+            output = u_boot_console.run_command(
+                '%swrite host 0:0 %x /dir1/%s.w8 0x1400 %x'
+                    % (fs_type, ADDR, MIN_FILE, 0x100000 + 0x1400))
+            assert('Unable to write "/dir1' in output)
+
+    def test_fs_ext9(self, u_boot_console, fs_obj_ext):
+        """
+        Test Case 9 - write to a non-existing file at non-zero offset
+        """
+        fs_type,fs_img,md5val = fs_obj_ext
+        with u_boot_console.log.section('Test Case 9 - write to non-existing file with non-zero offset'):
+            # Test Case 9a - Check if command expectedly failed
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%sload host 0:0 %x /%s' % (fs_type, ADDR, MIN_FILE),
+                '%swrite host 0:0 %x /dir1/%s.w9 0x1400 0x1400'
+                    % (fs_type, ADDR, MIN_FILE)])
+            assert('Unable to write "/dir1' in ''.join(output))
-- 
2.18.0

[U-Boot] [PATCH v2 23/23] test/py: fs: add fstest/mkdir test

[AKASHI Takahiro]

In this commit, test cases for mkdir interfaces are added as part of
"test_fs" test suite.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/tests/test_fs/conftest.py   |  31 ++++++++
 test/py/tests/test_fs/test_mkdir.py | 112 ++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+)
 create mode 100644 test/py/tests/test_fs/test_mkdir.py

diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
index f72d53d88fc8..0d909ed7789a 100644
--- a/test/py/tests/test_fs/conftest.py
+++ b/test/py/tests/test_fs/conftest.py
@@ -11,6 +11,7 @@ from fstest_defs import *
 
 supported_fs_basic = ['fat16', 'fat32', 'ext4']
 supported_fs_ext = ['fat16', 'fat32']
+supported_fs_mkdir = ['fat16', 'fat32']
 
 #
 # Filesystem test specific setup
@@ -22,6 +23,7 @@ def pytest_addoption(parser):
 def pytest_configure(config):
     global supported_fs_basic
     global supported_fs_ext
+    global supported_fs_mkdir
 
     def intersect(listA, listB):
         return  [x for x in listA if x in listB]
@@ -31,6 +33,7 @@ def pytest_configure(config):
         print("*** FS TYPE modified: %s" % supported_fs)
         supported_fs_basic =  intersect(supported_fs, supported_fs_basic)
         supported_fs_ext =  intersect(supported_fs, supported_fs_ext)
+        supported_fs_mkdir =  intersect(supported_fs, supported_fs_mkdir)
 
 def pytest_generate_tests(metafunc):
     if 'fs_obj_basic' in metafunc.fixturenames:
@@ -39,6 +42,9 @@ def pytest_generate_tests(metafunc):
     if 'fs_obj_ext' in metafunc.fixturenames:
         metafunc.parametrize('fs_obj_ext', supported_fs_ext,
             indirect=True, scope='module')
+    if 'fs_obj_mkdir' in metafunc.fixturenames:
+        metafunc.parametrize('fs_obj_mkdir', supported_fs_mkdir,
+            indirect=True, scope='module')
 
 #
 # Helper functions
@@ -298,3 +304,28 @@ def fs_obj_ext(request, u_boot_config):
         call('rmdir %s' % mount_dir, shell=True)
 #        if fs_img:
 #            call('rm -f %s' % fs_img, shell=True)
+
+
+#
+# Fixture for mkdir test
+#
+# NOTE: yield_fixture was deprecated since pytest-3.0
+ at pytest.yield_fixture()
+def fs_obj_mkdir(request, u_boot_config):
+    fs_type = request.param
+    fs_img = ''
+
+    fs_ubtype = fstype_to_ubname(fs_type)
+    check_ubconfig(u_boot_config, fs_ubtype)
+
+    try:
+        # 128MiB volume
+        fs_img = mk_fs(u_boot_config, fs_type, 0x8000000, '128MB')
+    except:
+        pytest.skip('Setup failed for filesystem: ' + fs_type)
+    else:
+        yield [fs_ubtype, fs_img]
+    finally:
+        print 'Dummy'
+#        if fs_img:
+#            call('rm -f %s' % fs_img, shell=True)
diff --git a/test/py/tests/test_fs/test_mkdir.py b/test/py/tests/test_fs/test_mkdir.py
new file mode 100644
index 000000000000..d9da97b56b5e
--- /dev/null
+++ b/test/py/tests/test_fs/test_mkdir.py
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier:      GPL-2.0+
+# Copyright (c) 2018, Linaro Limited
+# Author: Takahiro Akashi <takahiro.akashi@linaro.org>
+#
+# U-Boot File System:mkdir Test
+
+"""
+This test verifies mkdir operation on file system.
+"""
+
+import pytest
+
+ at pytest.mark.boardspec('sandbox')
+class TestMkdir(object):
+    def test_mkdir1(self, u_boot_console, fs_obj_mkdir):
+        """
+        Test Case 1 - create a directory under a root
+        """
+        fs_type,fs_img = fs_obj_mkdir
+        with u_boot_console.log.section('Test Case 1 - mkdir'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%smkdir host 0:0 dir1' % fs_type,
+                '%sls host 0:0 /' % fs_type])
+            assert('dir1/' in ''.join(output))
+
+            output = u_boot_console.run_command(
+                '%sls host 0:0 dir1' % fs_type)
+            assert('./'   in output)
+            assert('../'  in output)
+
+    def test_mkdir2(self, u_boot_console, fs_obj_mkdir):
+        """
+        Test Case 2 - create a directory under a sub-directory
+        """
+        fs_type,fs_img = fs_obj_mkdir
+        with u_boot_console.log.section('Test Case 2 - mkdir (sub-sub directory)'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%smkdir host 0:0 dir1/dir2' % fs_type,
+                '%sls host 0:0 dir1' % fs_type])
+            assert('dir2/' in ''.join(output))
+
+            output = u_boot_console.run_command(
+                '%sls host 0:0 dir1/dir2' % fs_type)
+            assert('./'   in output)
+            assert('../'  in output)
+
+    def test_mkdir3(self, u_boot_console, fs_obj_mkdir):
+        """
+        Test Case 3 - trying to create a directory with a non-existing
+        path should fail
+        """
+        fs_type,fs_img = fs_obj_mkdir
+        with u_boot_console.log.section('Test Case 3 - mkdir (non-existing path)'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%smkdir host 0:0 none/dir3' % fs_type])
+            assert('Unable to create a directory' in ''.join(output))
+
+    def test_mkdir4(self, u_boot_console, fs_obj_mkdir):
+        """
+        Test Case 4 - trying to create "." should fail
+        """
+        fs_type,fs_img = fs_obj_mkdir
+        with u_boot_console.log.section('Test Case 4 - mkdir (".")'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%smkdir host 0:0 .' % fs_type])
+            assert('Unable to create a directory' in ''.join(output))
+
+    def test_mkdir5(self, u_boot_console, fs_obj_mkdir):
+        """
+        Test Case 5 - trying to create ".." should fail
+        """
+        fs_type,fs_img = fs_obj_mkdir
+        with u_boot_console.log.section('Test Case 5 - mkdir ("..")'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%smkdir host 0:0 ..' % fs_type])
+            assert('Unable to create a directory' in ''.join(output))
+
+    def test_mkdir6(self, u_boot_console, fs_obj_mkdir):
+        """
+        'Test Case 6 - create as many directories as amount of directory
+        entries goes beyond a cluster size)'
+        """
+        fs_type,fs_img = fs_obj_mkdir
+        with u_boot_console.log.section('Test Case 6 - mkdir (create many)'):
+            output = u_boot_console.run_command_list([
+                'host bind 0 %s' % fs_img,
+                '%smkdir host 0:0 dir6' % fs_type,
+                '%sls host 0:0 /' % fs_type])
+            assert('dir6/' in ''.join(output))
+
+            for i in range(0, 20):
+                output = u_boot_console.run_command(
+                    '%smkdir host 0:0 dir6/0123456789abcdef%02x'
+                    % (fs_type, i))
+            output = u_boot_console.run_command('%sls host 0:0 dir6' % fs_type)
+            assert('0123456789abcdef00/'  in output)
+            assert('0123456789abcdef13/'  in output)
+
+            output = u_boot_console.run_command(
+                '%sls host 0:0 dir6/0123456789abcdef13/.' % fs_type)
+            assert('./'   in output)
+            assert('../'  in output)
+
+            output = u_boot_console.run_command(
+                '%sls host 0:0 dir6/0123456789abcdef13/..' % fs_type)
+            assert('0123456789abcdef00/'  in output)
+            assert('0123456789abcdef13/'  in output)
-- 
2.18.0

[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

[Alexander Graf]

On 04.09.18 09:49, AKASHI Takahiro wrote:
> The whole content of include/fat.h is private to FAT implementation
> and then should be guarded with CONFIG_FS_FAT.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/fat.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/fat.h b/include/fat.h
> index 09e142368585..c02839dcb040 100644
> --- a/include/fat.h
> +++ b/include/fat.h
> @@ -9,6 +9,8 @@
>  #ifndef _FAT_H_
>  #define _FAT_H_
>  
> +#ifdef CONFIG_FS_FAT

Isn't this missing an include of at least common.h to actually propagate
the config define?

Also, you want to use #if CONFIG_IS_ENABLED(...) here to guard against
SPL builds too.

However, I don't fully grasp why you need this patch. Please describe in
your commit message what the incentive is for creating it. What errors
are you trying to protect against?


Alex

> +
>  #include <asm/byteorder.h>
>  #include <fs.h>
>  
> @@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
>  int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
>  void fat_closedir(struct fs_dir_stream *dirs);
>  void fat_close(void);
> +#endif /* CONFIG_FS_FAT */
>  #endif /* _FAT_H_ */
> 

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[Alexander Graf]

On 04.09.18 09:49, AKASHI Takahiro wrote:
> Directory iterator was introduced in major re-work of read operation by
> Rob. We want to use it for write operation extensively as well.

Please indicate in the commit message that write operations are
implemented in a different .c file and so we have to export the
respective functions.

Alex

[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[Alexander Graf]

On 04.09.18 09:49, AKASHI Takahiro wrote:
> This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
> Returning EFI_SUCCESS can cheat SCT execution.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

How hard would it be to implement a real delete instead?


Alex

> ---
>  lib/efi_loader/efi_file.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index 6ec98c80227e..12044a0c7196 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -226,12 +226,20 @@ static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file)
>  	return EFI_EXIT(file_close(fh));
>  }
>  
> +static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file,
> +					  efi_uintn_t *buffer_size,
> +					  void *buffer);
> +
>  static efi_status_t EFIAPI efi_file_delete(struct efi_file_handle *file)
>  {
>  	struct file_handle *fh = to_fh(file);
> +	efi_uintn_t size = 0;
>  	EFI_ENTRY("%p", file);
> +
> +	/* TODO: implement real 'delete' */
> +	efi_file_write(file, &size, NULL);
>  	file_close(fh);
> -	return EFI_EXIT(EFI_WARN_DELETE_FAILURE);
> +	return EFI_SUCCESS;
>  }
>  
>  static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
> 

[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

[Heinrich Schuchardt]

On 09/04/2018 10:52 AM, Alexander Graf wrote:
> 
> 
> On 04.09.18 09:49, AKASHI Takahiro wrote:
>> The whole content of include/fat.h is private to FAT implementation
>> and then should be guarded with CONFIG_FS_FAT.

Hello Takahiro,

doesn't this imply that building common/spl/spl_sata.c without FAT will
fail for CONFIG_SPL_SATA_SUPPORT=y (e.g. cm_t54_defconfig with FAT
disabled).

Did you run Travis CI on your patch series?

>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> ---
>>  include/fat.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/fat.h b/include/fat.h
>> index 09e142368585..c02839dcb040 100644
>> --- a/include/fat.h
>> +++ b/include/fat.h
>> @@ -9,6 +9,8 @@
>>  #ifndef _FAT_H_
>>  #define _FAT_H_
>>  
>> +#ifdef CONFIG_FS_FAT
> 
> Isn't this missing an include of at least common.h to actually propagate
> the config define?
> 
> Also, you want to use #if CONFIG_IS_ENABLED(...) here to guard against
> SPL builds too.

Probably not:
common/spl/spl_fat.c:14:#include <fat.h>

Best regards

Heinrich Schuchardt

> 
> However, I don't fully grasp why you need this patch. Please describe in
> your commit message what the incentive is for creating it. What errors
> are you trying to protect against?
> 
> 
> Alex
> 
>> +
>>  #include <asm/byteorder.h>
>>  #include <fs.h>
>>  
>> @@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
>>  int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
>>  void fat_closedir(struct fs_dir_stream *dirs);
>>  void fat_close(void);
>> +#endif /* CONFIG_FS_FAT */
>>  #endif /* _FAT_H_ */
>>
> 

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[Heinrich Schuchardt]

On 09/04/2018 11:01 AM, Alexander Graf wrote:
> 
> 
> On 04.09.18 09:49, AKASHI Takahiro wrote:
>> Directory iterator was introduced in major re-work of read operation by
>> Rob. We want to use it for write operation extensively as well.
> 
> Please indicate in the commit message that write operations are
> implemented in a different .c file and so we have to export the
> respective functions.

Why? Look at this ugly code:

fs/fat/fat_write.c:17:#include "fat.c"

Best regards

Heinrich

> 
> Alex
> 

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[Alexander Graf]

> Am 04.09.2018 um 12:50 schrieb Heinrich Schuchardt <xypron.glpk@gmx.de>:
> 
> 
> 
>> On 09/04/2018 11:01 AM, Alexander Graf wrote:
>> 
>> 
>>> On 04.09.18 09:49, AKASHI Takahiro wrote:
>>> Directory iterator was introduced in major re-work of read operation by
>>> Rob. We want to use it for write operation extensively as well.
>> 
>> Please indicate in the commit message that write operations are
>> implemented in a different .c file and so we have to export the
>> respective functions.
> 
> Why? Look at this ugly code:
> 
> fs/fat/fat_write.c:17:#include "fat.c"

In that case we don't need this patch at all, no?

Alex

> 
> Best regards
> 
> Heinrich
> 
>> 
>> Alex
>> 

[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

[AKASHI Takahiro]

On Tue, Sep 04, 2018 at 10:52:20AM +0200, Alexander Graf wrote:
> 
> 
> On 04.09.18 09:49, AKASHI Takahiro wrote:
> > The whole content of include/fat.h is private to FAT implementation
> > and then should be guarded with CONFIG_FS_FAT.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  include/fat.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/include/fat.h b/include/fat.h
> > index 09e142368585..c02839dcb040 100644
> > --- a/include/fat.h
> > +++ b/include/fat.h
> > @@ -9,6 +9,8 @@
> >  #ifndef _FAT_H_
> >  #define _FAT_H_
> >  
> > +#ifdef CONFIG_FS_FAT
> 
> Isn't this missing an include of at least common.h to actually propagate
> the config define?
> 
> Also, you want to use #if CONFIG_IS_ENABLED(...) here to guard against
> SPL builds too.

Let's discuss in a reply to Heinrich's comment.

> However, I don't fully grasp why you need this patch. Please describe in
> your commit message what the incentive is for creating it. What errors
> are you trying to protect against?

Sure; See
https://lists.denx.de/pipermail/u-boot/2018-August/338054.html

In short, one macro definition in this file will break without CONFI_FS_FAT_*.

Thanks,
-Takahiro AKASHI

> 
> Alex
> 
> > +
> >  #include <asm/byteorder.h>
> >  #include <fs.h>
> >  
> > @@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
> >  int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
> >  void fat_closedir(struct fs_dir_stream *dirs);
> >  void fat_close(void);
> > +#endif /* CONFIG_FS_FAT */
> >  #endif /* _FAT_H_ */
> > 

[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

[AKASHI Takahiro]

Hi Heinrich, Alex,

On Tue, Sep 04, 2018 at 12:46:58PM +0200, Heinrich Schuchardt wrote:
> 
> 
> On 09/04/2018 10:52 AM, Alexander Graf wrote:
> > 
> > 
> > On 04.09.18 09:49, AKASHI Takahiro wrote:
> >> The whole content of include/fat.h is private to FAT implementation
> >> and then should be guarded with CONFIG_FS_FAT.
> 
> Hello Takahiro,
> 
> doesn't this imply that building common/spl/spl_sata.c without FAT will
> fail for CONFIG_SPL_SATA_SUPPORT=y (e.g. cm_t54_defconfig with FAT
> disabled).

Yes, you're right, but this is a matter of cm_t54_defconfig.
spl_sta.c uses spl_load_image_fat[_os]() which are implemented
in spl_fat.c which replies on CONFIG_SPL_FAT_SUPPORT, which
will in turn select CONFIG_FS_FAT.
The problem is that cm_t54_defconfig doesn't enable
CONFIG_SPL_FAT_SUPPORT.

To fix this issue, we should add "depends on SPL_FAT_SUPPORT"
to SPL_SATA_SUPPORT (and SPL_USB_SUPPORT) as well.

> Did you run Travis CI on your patch series?

No. Is everybody required to run Travis CI for u-boot
before any submission?

> >>
> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >> ---
> >>  include/fat.h | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/include/fat.h b/include/fat.h
> >> index 09e142368585..c02839dcb040 100644
> >> --- a/include/fat.h
> >> +++ b/include/fat.h
> >> @@ -9,6 +9,8 @@
> >>  #ifndef _FAT_H_
> >>  #define _FAT_H_
> >>  
> >> +#ifdef CONFIG_FS_FAT
> > 
> > Isn't this missing an include of at least common.h to actually propagate
> > the config define?
> > 
> > Also, you want to use #if CONFIG_IS_ENABLED(...) here to guard against
> > SPL builds too.
> 
> Probably not:

As I'm saying, CONFIG_SPL_FAT_SUPPORT selects CONFIG_FS_FAT
and so either "ifdef CONFIG_FS_FAT" or "if  CONFIG_IS_ENABLED(FS_FAT)" will
turn into True although CONFIG_SPL_FS_FAT actually doesn't exist.

Therefore, I don't think we need a change Alex suggested.

Thanks,
-Takahiro AKASHI

> common/spl/spl_fat.c:14:#include <fat.h>
> 
> Best regards
> 
> Heinrich Schuchardt
> 
> > 
> > However, I don't fully grasp why you need this patch. Please describe in
> > your commit message what the incentive is for creating it. What errors
> > are you trying to protect against?
> > 
> > 
> > Alex
> > 
> >> +
> >>  #include <asm/byteorder.h>
> >>  #include <fs.h>
> >>  
> >> @@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp);
> >>  int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
> >>  void fat_closedir(struct fs_dir_stream *dirs);
> >>  void fat_close(void);
> >> +#endif /* CONFIG_FS_FAT */
> >>  #endif /* _FAT_H_ */
> >>
> > 

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[AKASHI Takahiro]

On Tue, Sep 04, 2018 at 12:57:54PM +0200, Alexander Graf wrote:
> 
> 
> > Am 04.09.2018 um 12:50 schrieb Heinrich Schuchardt <xypron.glpk@gmx.de>:
> > 
> > 
> > 
> >> On 09/04/2018 11:01 AM, Alexander Graf wrote:
> >> 
> >> 
> >>> On 04.09.18 09:49, AKASHI Takahiro wrote:
> >>> Directory iterator was introduced in major re-work of read operation by
> >>> Rob. We want to use it for write operation extensively as well.
> >> 
> >> Please indicate in the commit message that write operations are
> >> implemented in a different .c file and so we have to export the
> >> respective functions.
> > 
> > Why? Look at this ugly code:
> > 
> > fs/fat/fat_write.c:17:#include "fat.c"
> 
> In that case we don't need this patch at all, no?

Oops, I didn't notice this before.
If, however, "include fat.c" makes any sense, theoretically we don't need
"depends on FS_FAT" for FS_FAT_WRITE.
There seems to be a contradiction between the code and config.

I prefer just to remove the line, '#include "fat.c"' from fat_write.c
and add more "extern" definitions in fat.h if necessary.

Thanks,
-Takakahiro AKASHI

> Alex
> 
> > 
> > Best regards
> > 
> > Heinrich
> > 
> >> 
> >> Alex
> >> 

[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[AKASHI Takahiro]

On Tue, Sep 04, 2018 at 11:16:38AM +0200, Alexander Graf wrote:
> 
> 
> On 04.09.18 09:49, AKASHI Takahiro wrote:
> > This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
> > Returning EFI_SUCCESS can cheat SCT execution.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> How hard would it be to implement a real delete instead?

Good question.
The hardest part of implementation, I believe, is about handling
nasty long file names in a directory, in particular, when
directory entries which comprise a single LFN extend across two clusters.
In this case, all the entries must be deleted first, and then go back
to the first one and modify it. Due to a current simple buffering of
cluster (only one cluster buffer a time), some tweak is necessary.
In addition, if there is an already-deleted entry ahead of the given file,
we need further rewind directory entries and update the "new" last valid
one to mark it as so.
Those kind of things would make the implementation a bit complicated
rather than simple as expected.

So unless 'real' delete is a must for anything, I'm lukewarm to
work on it.

# I admit that we don't have to have this patch just if *delete* returns
successfully without doing anything.

Thanks,
-Takahiro AKASHI

> 
> Alex
> 
> > ---
> >  lib/efi_loader/efi_file.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> > index 6ec98c80227e..12044a0c7196 100644
> > --- a/lib/efi_loader/efi_file.c
> > +++ b/lib/efi_loader/efi_file.c
> > @@ -226,12 +226,20 @@ static efi_status_t EFIAPI efi_file_close(struct efi_file_handle *file)
> >  	return EFI_EXIT(file_close(fh));
> >  }
> >  
> > +static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file,
> > +					  efi_uintn_t *buffer_size,
> > +					  void *buffer);
> > +
> >  static efi_status_t EFIAPI efi_file_delete(struct efi_file_handle *file)
> >  {
> >  	struct file_handle *fh = to_fh(file);
> > +	efi_uintn_t size = 0;
> >  	EFI_ENTRY("%p", file);
> > +
> > +	/* TODO: implement real 'delete' */
> > +	efi_file_write(file, &size, NULL);
> >  	file_close(fh);
> > -	return EFI_EXIT(EFI_WARN_DELETE_FAILURE);
> > +	return EFI_SUCCESS;
> >  }
> >  
> >  static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
> > 

[U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h

[Heinrich Schuchardt]

On 09/05/2018 03:54 AM, AKASHI Takahiro wrote:
>> Did you run Travis CI on your patch series?
> No. Is everybody required to run Travis CI for u-boot
> before any submission?
> 

The custodian (Alex) will run Travis CI tests and if they fail you don't
get your patch series merged.

So it is advisable for the submitter of the patch series to already run
the same tests, especially for large changes.

This will require that you have a repository on Github and authorize
Travis CI to access it. Then every time you push to the repository the
Travis test suite will be run.

As this takes several hours I have decided to use a separate repository
for Travis testing only.

Best regards

Heinrich

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[Alexander Graf]

On 05.09.18 04:14, AKASHI Takahiro wrote:
> On Tue, Sep 04, 2018 at 12:57:54PM +0200, Alexander Graf wrote:
>>
>>
>>> Am 04.09.2018 um 12:50 schrieb Heinrich Schuchardt <xypron.glpk@gmx.de>:
>>>
>>>
>>>
>>>> On 09/04/2018 11:01 AM, Alexander Graf wrote:
>>>>
>>>>
>>>>> On 04.09.18 09:49, AKASHI Takahiro wrote:
>>>>> Directory iterator was introduced in major re-work of read operation by
>>>>> Rob. We want to use it for write operation extensively as well.
>>>>
>>>> Please indicate in the commit message that write operations are
>>>> implemented in a different .c file and so we have to export the
>>>> respective functions.
>>>
>>> Why? Look at this ugly code:
>>>
>>> fs/fat/fat_write.c:17:#include "fat.c"
>>
>> In that case we don't need this patch at all, no?
> 
> Oops, I didn't notice this before.
> If, however, "include fat.c" makes any sense, theoretically we don't need
> "depends on FS_FAT" for FS_FAT_WRITE.
> There seems to be a contradiction between the code and config.

I'm not sure it's contradictory. Someone just implemented a poor man's
LTO by including the 2 files with each other.

> I prefer just to remove the line, '#include "fat.c"' from fat_write.c
> and add more "extern" definitions in fat.h if necessary.

We can worry about refactoring things later down the road. By making
functions go external, we lose the compiler's ability to inline
functions which may again bloat code which may trip random boards above
size limits.

For now, I'd suggest you leave the ugly #include "fat.c" in and not move
anything into fat.h. That way the file is self-contained and you don't
have to worry about exporting just yet.


Alex

[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[Alexander Graf]

On 05.09.18 04:51, AKASHI Takahiro wrote:
> On Tue, Sep 04, 2018 at 11:16:38AM +0200, Alexander Graf wrote:
>>
>>
>> On 04.09.18 09:49, AKASHI Takahiro wrote:
>>> This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
>>> Returning EFI_SUCCESS can cheat SCT execution.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>
>> How hard would it be to implement a real delete instead?
> 
> Good question.
> The hardest part of implementation, I believe, is about handling
> nasty long file names in a directory, in particular, when
> directory entries which comprise a single LFN extend across two clusters.
> In this case, all the entries must be deleted first, and then go back
> to the first one and modify it. Due to a current simple buffering of
> cluster (only one cluster buffer a time), some tweak is necessary.
> In addition, if there is an already-deleted entry ahead of the given file,
> we need further rewind directory entries and update the "new" last valid
> one to mark it as so.
> Those kind of things would make the implementation a bit complicated
> rather than simple as expected.
> 
> So unless 'real' delete is a must for anything, I'm lukewarm to
> work on it.
> 
> # I admit that we don't have to have this patch just if *delete* returns
> successfully without doing anything.

I think both truncate-to-zero (this patch) and
report-success-when-failure (suggested patch) are hacks that shouldn't
really go upstream that way unfortunately.

The way I read the long file name definitions, I think it should be
perfectly ok to only remove the short file name directory entry. Sure,
we're leaking a few directory entries for the LFN, but I don't think
that's too bad. Any fsck should be able to find those later on...

As for how to delete the entry without rewinding directory entries, you
can just set name[0]=DELETED_FLAG; to indicate that the SFN is deleted, no?


Alex

[U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use

[AKASHI Takahiro]

On Wed, Sep 05, 2018 at 10:16:32AM +0200, Alexander Graf wrote:
> 
> 
> On 05.09.18 04:14, AKASHI Takahiro wrote:
> > On Tue, Sep 04, 2018 at 12:57:54PM +0200, Alexander Graf wrote:
> >>
> >>
> >>> Am 04.09.2018 um 12:50 schrieb Heinrich Schuchardt <xypron.glpk@gmx.de>:
> >>>
> >>>
> >>>
> >>>> On 09/04/2018 11:01 AM, Alexander Graf wrote:
> >>>>
> >>>>
> >>>>> On 04.09.18 09:49, AKASHI Takahiro wrote:
> >>>>> Directory iterator was introduced in major re-work of read operation by
> >>>>> Rob. We want to use it for write operation extensively as well.
> >>>>
> >>>> Please indicate in the commit message that write operations are
> >>>> implemented in a different .c file and so we have to export the
> >>>> respective functions.
> >>>
> >>> Why? Look at this ugly code:
> >>>
> >>> fs/fat/fat_write.c:17:#include "fat.c"
> >>
> >> In that case we don't need this patch at all, no?
> > 
> > Oops, I didn't notice this before.
> > If, however, "include fat.c" makes any sense, theoretically we don't need
> > "depends on FS_FAT" for FS_FAT_WRITE.
> > There seems to be a contradiction between the code and config.
> 
> I'm not sure it's contradictory. Someone just implemented a poor man's
> LTO by including the 2 files with each other.

Well, LTO has long existed since gcc4.8 ...
(and improvement would be quite small, I guess.)

> > I prefer just to remove the line, '#include "fat.c"' from fat_write.c
> > and add more "extern" definitions in fat.h if necessary.
> 
> We can worry about refactoring things later down the road. By making
> functions go external, we lose the compiler's ability to inline
> functions which may again bloat code which may trip random boards above
> size limits.
> 
> For now, I'd suggest you leave the ugly #include "fat.c" in and not move
> anything into fat.h. That way the file is self-contained and you don't
> have to worry about exporting just yet.

Although I won't tolerate such an ugly code, I will follow your suggestion
mainly because lots of existing defconfigs enable FAT_WRITE without
FS_FAT.

Thanks,
-Takahiro AKASHI

> 
> Alex

[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[AKASHI Takahiro]

On Wed, Sep 05, 2018 at 10:22:07AM +0200, Alexander Graf wrote:
> 
> 
> On 05.09.18 04:51, AKASHI Takahiro wrote:
> > On Tue, Sep 04, 2018 at 11:16:38AM +0200, Alexander Graf wrote:
> >>
> >>
> >> On 04.09.18 09:49, AKASHI Takahiro wrote:
> >>> This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
> >>> Returning EFI_SUCCESS can cheat SCT execution.
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>
> >> How hard would it be to implement a real delete instead?
> > 
> > Good question.
> > The hardest part of implementation, I believe, is about handling
> > nasty long file names in a directory, in particular, when
> > directory entries which comprise a single LFN extend across two clusters.
> > In this case, all the entries must be deleted first, and then go back
> > to the first one and modify it. Due to a current simple buffering of
> > cluster (only one cluster buffer a time), some tweak is necessary.
> > In addition, if there is an already-deleted entry ahead of the given file,
> > we need further rewind directory entries and update the "new" last valid
> > one to mark it as so.
> > Those kind of things would make the implementation a bit complicated
> > rather than simple as expected.
> > 
> > So unless 'real' delete is a must for anything, I'm lukewarm to
> > work on it.
> > 
> > # I admit that we don't have to have this patch just if *delete* returns
> > successfully without doing anything.
> 
> I think both truncate-to-zero (this patch) and
> report-success-when-failure (suggested patch) are hacks that shouldn't
> really go upstream that way unfortunately.
> 
> The way I read the long file name definitions, I think it should be
> perfectly ok to only remove the short file name directory entry.

Do you accept such an ugly implementation although it yet complies
with fat specification?

> Sure,
> we're leaking a few directory entries for the LFN, but I don't think
> that's too bad. Any fsck should be able to find those later on...

As I said in my cover-letter, fsck would complain any way.

> As for how to delete the entry without rewinding directory entries, you
> can just set name[0]=DELETED_FLAG; to indicate that the SFN is deleted, no?

Yes if DELETED_FLAG=0x5e and if we ignore the specific case where
name[0] be 0x00, which means the entry is the *first* invalid one.
(again, the latter would not be mandatory in term of compatibility.)

Since I have already had the code, "real" delete will be put in my next
version if nobody has concerns on this simple implementation.

Thanks,
-Takahiro AKASHI

> 
> Alex

[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[Alexander Graf]

> Am 06.09.2018 um 04:43 schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> 
>> On Wed, Sep 05, 2018 at 10:22:07AM +0200, Alexander Graf wrote:
>> 
>> 
>>> On 05.09.18 04:51, AKASHI Takahiro wrote:
>>>> On Tue, Sep 04, 2018 at 11:16:38AM +0200, Alexander Graf wrote:
>>>> 
>>>> 
>>>>> On 04.09.18 09:49, AKASHI Takahiro wrote:
>>>>> This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
>>>>> Returning EFI_SUCCESS can cheat SCT execution.
>>>>> 
>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>>> 
>>>> How hard would it be to implement a real delete instead?
>>> 
>>> Good question.
>>> The hardest part of implementation, I believe, is about handling
>>> nasty long file names in a directory, in particular, when
>>> directory entries which comprise a single LFN extend across two clusters.
>>> In this case, all the entries must be deleted first, and then go back
>>> to the first one and modify it. Due to a current simple buffering of
>>> cluster (only one cluster buffer a time), some tweak is necessary.
>>> In addition, if there is an already-deleted entry ahead of the given file,
>>> we need further rewind directory entries and update the "new" last valid
>>> one to mark it as so.
>>> Those kind of things would make the implementation a bit complicated
>>> rather than simple as expected.
>>> 
>>> So unless 'real' delete is a must for anything, I'm lukewarm to
>>> work on it.
>>> 
>>> # I admit that we don't have to have this patch just if *delete* returns
>>> successfully without doing anything.
>> 
>> I think both truncate-to-zero (this patch) and
>> report-success-when-failure (suggested patch) are hacks that shouldn't
>> really go upstream that way unfortunately.
>> 
>> The way I read the long file name definitions, I think it should be
>> perfectly ok to only remove the short file name directory entry.
> 
> Do you accept such an ugly implementation although it yet complies
> with fat specification?

It's valid for all intents and purposes, so why not :).

> 
>> Sure,
>> we're leaking a few directory entries for the LFN, but I don't think
>> that's too bad. Any fsck should be able to find those later on...
> 
> As I said in my cover-letter, fsck would complain any way.
> 
>> As for how to delete the entry without rewinding directory entries, you
>> can just set name[0]=DELETED_FLAG; to indicate that the SFN is deleted, no?
> 
> Yes if DELETED_FLAG=0x5e and if we ignore the specific case where
> name[0] be 0x00, which means the entry is the *first* invalid one.
> (again, the latter would not be mandatory in term of compatibility.)
> 
> Since I have already had the code, "real" delete will be put in my next
> version if nobody has concerns on this simple implementation.

Thanks, that would be great! :)

Alex

[U-Boot] [PATCH v2 21/23] test/py: convert fs-test.sh to pytest

[Simon Glass]

Hi Akashi,

On 4 September 2018 at 09:49, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
> In this commit, the same set of test cases as in test/fs/fs-test.sh
> is provided using pytest framework.
> Actually, fs-test.sh provides three variants:"sb" (sb command), "nonfs"
> (fatxx and etc.) and "fs" (hostfs), and this patch currently supports
> only "nonfs" variant; So it is not a replacement of fs-test.sh for now.
>
> Simple usage:
>   $ py.test test/py/tests/test_fs [<other options>]
>
> You may also specify filesystem types to be tested:
>   $ py.test test/py/tests/test_fs --fs-type fat32 [<other options>]
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  test/py/tests/test_fs/conftest.py    | 216 ++++++++++++++++++++
>  test/py/tests/test_fs/fstest_defs.py |  10 +
>  test/py/tests/test_fs/test_basic.py  | 287 +++++++++++++++++++++++++++
>  3 files changed, 513 insertions(+)
>  create mode 100644 test/py/tests/test_fs/conftest.py
>  create mode 100644 test/py/tests/test_fs/fstest_defs.py
>  create mode 100644 test/py/tests/test_fs/test_basic.py

Thanks for doing this!

Can you also please delete the old shell script?

Does this get automatically executed with 'make tests'?

If not, is it possible to do that easily, if we reduce the size of files, etc?

>
> diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
> new file mode 100644
> index 000000000000..9155ccf84266
> --- /dev/null
> +++ b/test/py/tests/test_fs/conftest.py
> @@ -0,0 +1,216 @@
> +# SPDX-License-Identifier:      GPL-2.0+
> +# Copyright (c) 2018, Linaro Limited
> +# Author: Takahiro Akashi <takahiro.akashi@linaro.org>
> +
> +import os
> +import os.path
> +import pytest
> +import re
> +from subprocess import call, check_call, check_output, CalledProcessError
> +from fstest_defs import *
> +
> +supported_fs_basic = ['fat16', 'fat32', 'ext4']
> +
> +#
> +# Filesystem test specific setup
> +#
> +def pytest_addoption(parser):

Please can you add full function comments to each function? You can
see other tests or binman for the format to use.
[...]

Regards,
Simon

[U-Boot] [PATCH v2 21/23] test/py: convert fs-test.sh to pytest

[AKASHI Takahiro]

Simon,

Sorry for not responding soon.

On Fri, Sep 14, 2018 at 12:54:57PM +0200, Simon Glass wrote:
> Hi Akashi,
> 
> On 4 September 2018 at 09:49, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> > In this commit, the same set of test cases as in test/fs/fs-test.sh
> > is provided using pytest framework.
> > Actually, fs-test.sh provides three variants:"sb" (sb command), "nonfs"
> > (fatxx and etc.) and "fs" (hostfs), and this patch currently supports
> > only "nonfs" variant; So it is not a replacement of fs-test.sh for now.
> >
> > Simple usage:
> >   $ py.test test/py/tests/test_fs [<other options>]
> >
> > You may also specify filesystem types to be tested:
> >   $ py.test test/py/tests/test_fs --fs-type fat32 [<other options>]
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  test/py/tests/test_fs/conftest.py    | 216 ++++++++++++++++++++
> >  test/py/tests/test_fs/fstest_defs.py |  10 +
> >  test/py/tests/test_fs/test_basic.py  | 287 +++++++++++++++++++++++++++
> >  3 files changed, 513 insertions(+)
> >  create mode 100644 test/py/tests/test_fs/conftest.py
> >  create mode 100644 test/py/tests/test_fs/fstest_defs.py
> >  create mode 100644 test/py/tests/test_fs/test_basic.py
> 
> Thanks for doing this!
> 
> Can you also please delete the old shell script?

No, we can't.
As I said in the commit message, my script currently supports
only one of three test variants in fs-test.sh, "nonfs."
"sb" and "fs" are yet to be implemented (if necessary).

> Does this get automatically executed with 'make tests'?

Yes, of course.

> If not, is it possible to do that easily, if we reduce the size of files, etc?
> 
> >
> > diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
> > new file mode 100644
> > index 000000000000..9155ccf84266
> > --- /dev/null
> > +++ b/test/py/tests/test_fs/conftest.py
> > @@ -0,0 +1,216 @@
> > +# SPDX-License-Identifier:      GPL-2.0+
> > +# Copyright (c) 2018, Linaro Limited
> > +# Author: Takahiro Akashi <takahiro.akashi@linaro.org>
> > +
> > +import os
> > +import os.path
> > +import pytest
> > +import re
> > +from subprocess import call, check_call, check_output, CalledProcessError
> > +from fstest_defs import *
> > +
> > +supported_fs_basic = ['fat16', 'fat32', 'ext4']
> > +
> > +#
> > +# Filesystem test specific setup
> > +#
> > +def pytest_addoption(parser):
> 
> Please can you add full function comments to each function? You can
> see other tests or binman for the format to use.
> [...]

OK.

-Takahiro Akashi


> Regards,
> Simon