[U-Boot] [PATCH 00/17] fs: fat: extend FAT write operations
This patch series 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
Patch#1 to patch#6 are some sort of preparatory ones. Patch#7 implements write with sub-directories path. Patch#8 to patch#10 implement write with non-zero offset. Patch#11 to patch#15 are related to creating a directory. Patch#16 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. Finally, patch#17 fixes a minor bug in fs-test.sh.
I applied this patch set on top of v2018.07 along with a couple of yet-to--be-upstreamed UEFI-related patches, and could successfully run unmodified SCT[1] on qemu-arm.
AKASHI Takahiro (17): 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 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
cmd/fat.c | 22 +- fs/fat/fat.c | 98 ++-- fs/fat/fat_write.c | 1051 +++++++++++++++++++++++-------------- fs/fs.c | 46 ++ include/fat.h | 37 ++ include/fs.h | 10 + lib/efi_loader/efi_file.c | 24 +- test/fs/fs-test.sh | 2 +- 8 files changed, 825 insertions(+), 465 deletions(-)
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 | 3 +++ include/fat.h | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 4efe8a3eda..b48f48a751 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -556,6 +556,9 @@ static int get_fs_info(fsdata *mydata) return ret; }
+ mydata->bs_total_sect = bs.total_sect; + mydata->bs_fats = bs.fats; + if (mydata->fatsize == 32) { mydata->fatlength = bs.fat32_length; } else { diff --git a/include/fat.h b/include/fat.h index 09e1423685..0c88b59a4a 100644 --- a/include/fat.h +++ b/include/fat.h @@ -173,6 +173,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 bs_total_sect; /* Boot sector's number of sectors */ + __u8 bs_fats; /* Boot sector's number of FATs */ } fsdata;
static inline u32 clust_to_sect(fsdata *fsdata, u32 clust)
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 | 3 +++ include/fat.h | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 4efe8a3eda..b48f48a751 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -556,6 +556,9 @@ static int get_fs_info(fsdata *mydata) return ret; }
- mydata->bs_total_sect = bs.total_sect;
- mydata->bs_fats = bs.fats;
- if (mydata->fatsize == 32) { mydata->fatlength = bs.fat32_length; } else {
diff --git a/include/fat.h b/include/fat.h index 09e1423685..0c88b59a4a 100644 --- a/include/fat.h +++ b/include/fat.h @@ -173,6 +173,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 bs_total_sect; /* Boot sector's number of sectors */
How can one sector have multiple sectors? Please, reword the comment to something more obvious.
Best regards
Heinrich
- __u8 bs_fats; /* Boot sector's number of FATs */
} fsdata;
static inline u32 clust_to_sect(fsdata *fsdata, u32 clust)
On 07/20/2018 08:06 PM, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 | 3 +++ include/fat.h | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 4efe8a3eda..b48f48a751 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -556,6 +556,9 @@ static int get_fs_info(fsdata *mydata) return ret; }
- mydata->bs_total_sect = bs.total_sect;
Please, check here if sectors is non-zero and in this case use that value. Do not rely on total_sect = 0 in later coding to read sectors. Anyway there should be only one place in the coding where we determine the sector count.
The same logic is used in the Linux kernel:
fs/fat/inode.c:1766: total_sectors = bpb.fat_sectors; fs/fat/inode.c:1767: if (total_sectors == 0) fs/fat/inode.c:1768: total_sectors = bpb.fat_total_sect;
- mydata->bs_fats = bs.fats;
- if (mydata->fatsize == 32) { mydata->fatlength = bs.fat32_length; } else {
diff --git a/include/fat.h b/include/fat.h index 09e1423685..0c88b59a4a 100644 --- a/include/fat.h +++ b/include/fat.h @@ -173,6 +173,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 bs_total_sect; /* Boot sector's number of sectors */
Please, use
__u32 total_sect; /* Number of sectors */
How can one sector have multiple sectors? Please, reword the comment to something more obvious.
Best regards
Heinrich
- __u8 bs_fats; /* Boot sector's number of FATs */
and
__u8 fats; /* Number of FATs */
This is not information about the boot sector but about the partition.
Best regards
Heinich
} fsdata;
static inline u32 clust_to_sect(fsdata *fsdata, u32 clust)
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 b48f48a751..fd6523c66b 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -927,6 +927,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));
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 b48f48a751..fd6523c66b 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -927,6 +927,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 ".." */
I understand why root has no ../ but /./ should be valid.
On Linux 'ls /./' displays the root directory.
Best regards
Heinrich
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;elsereturn -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));
On Fri, Jul 20, 2018 at 08:09:00PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 b48f48a751..fd6523c66b 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -927,6 +927,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 ".." */I understand why root has no ../ but /./ should be valid.
What I meant here is that, according to FAT specification, the root directory has neither "." directory entry nor ".." in its associated clusters in contrast to ordinary directories. So when user specifies "." or ".." (and other variants), we will have to take special care to handle them correctly.
On Linux 'ls /./' displays the root directory.
My code works with any of the following paths: * . * .. * / * /. * /.., or * even any combination of above, like ./.././.././
Note that "ls /.." on linux also shows the root directory.
Please try.
-Takahiro AKASHI
Best regards
Heinrich
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;elsereturn -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));
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 defition, 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 fd6523c66b..0f82cbe1bd 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -634,25 +634,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 @@ -661,7 +642,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; @@ -693,7 +674,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); @@ -713,7 +694,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; @@ -834,7 +815,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;
@@ -879,19 +860,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. @@ -907,7 +880,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 0c88b59a4a..577e6b4592 100644 --- a/include/fat.h +++ b/include/fat.h @@ -187,6 +187,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);
On 07/20/2018 04:57 AM, 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. This patch makes relevant functions, as well as iterator defition, 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 fd6523c66b..0f82cbe1bd 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -634,25 +634,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
@@ -661,7 +642,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; @@ -693,7 +674,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); @@ -713,7 +694,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; @@ -834,7 +815,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;
@@ -879,19 +860,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.
@@ -907,7 +880,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 0c88b59a4a..577e6b4592 100644 --- a/include/fat.h +++ b/include/fat.h @@ -187,6 +187,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)
Please, rename these to something more specific like FS_ITER_ANY.
TYPE_ANY already is defined in fs/reiserfs/reiserfs_private.h:
fs/reiserfs/reiserfs_private.h:160:#define TYPE_ANY 15
Best regards
Heinrich
+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);
On Fri, Jul 20, 2018 at 08:02:57PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, 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. This patch makes relevant functions, as well as iterator defition, 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 fd6523c66b..0f82cbe1bd 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -634,25 +634,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
@@ -661,7 +642,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; @@ -693,7 +674,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); @@ -713,7 +694,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; @@ -834,7 +815,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;
@@ -879,19 +860,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.
@@ -907,7 +880,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 0c88b59a4a..577e6b4592 100644 --- a/include/fat.h +++ b/include/fat.h @@ -187,6 +187,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)
Please, rename these to something more specific like FS_ITER_ANY.
TYPE_ANY already is defined in fs/reiserfs/reiserfs_private.h:
fs/reiserfs/reiserfs_private.h:160:#define TYPE_ANY 15
Good catch though the definition above comes directly from Rob's original patch.
Since there is only one occurrence of TYPE_ANY in fs/fat, I'd rather expand it without introducing another definition.
Thanks, -Takahiro AKASHI
Best regards
Heinrich
+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);
On Mon, Jul 23, 2018 at 05:06:46PM +0900, AKASHI Takahiro wrote:
On Fri, Jul 20, 2018 at 08:02:57PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, 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. This patch makes relevant functions, as well as iterator defition, 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 fd6523c66b..0f82cbe1bd 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -634,25 +634,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
@@ -661,7 +642,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; @@ -693,7 +674,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); @@ -713,7 +694,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; @@ -834,7 +815,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;
@@ -879,19 +860,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.
@@ -907,7 +880,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 0c88b59a4a..577e6b4592 100644 --- a/include/fat.h +++ b/include/fat.h @@ -187,6 +187,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)
Please, rename these to something more specific like FS_ITER_ANY.
TYPE_ANY already is defined in fs/reiserfs/reiserfs_private.h:
fs/reiserfs/reiserfs_private.h:160:#define TYPE_ANY 15
Good catch though the definition above comes directly from Rob's original patch.
Oops, looking at the code closely, the duplication will never be hit because reiserfs_private.h is a private header.
Since there is only one occurrence of TYPE_ANY in fs/fat, I'd rather expand it without introducing another definition.
I will put this change on hold unless a maintainer sticks to this idea.
Thanks, -Takahiro AKASHI
Best regards
Heinrich
+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);
On 07/20/2018 04:57 AM, 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. This patch makes relevant functions, as well as iterator defition, 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 fd6523c66b..0f82cbe1bd 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -634,25 +634,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
@@ -661,7 +642,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; @@ -693,7 +674,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); @@ -713,7 +694,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; @@ -834,7 +815,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;
@@ -879,19 +860,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.
@@ -907,7 +880,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 0c88b59a4a..577e6b4592 100644 --- a/include/fat.h +++ b/include/fat.h @@ -187,6 +187,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);
If CONFIG_FAT is not set, MAX_CLUSTSIZE is empty. And you get a build error:
CC fs/fs.o In file included from fs/fs.c:12:0: include/fat.h:20:23: error: ‘CONFIG_FS_FAT_MAX_CLUSTSIZE’ undeclared here (not in a function); did you mean ‘CONFIG_SYS_MAX_FLASH_SECT’? #define MAX_CLUSTSIZE CONFIG_FS_FAT_MAX_CLUSTSIZE ^ include/fat.h:214:19: note: in expansion of macro ‘MAX_CLUSTSIZE’ u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); ^~~~~~~~~~~~~ make[1]: *** [scripts/Makefile.build:279: fs/fs.o] Error 1
We should not assume that every board uses FAT.
Best regards
Heinrich
+} 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);
On Sat, Aug 11, 2018 at 03:34:20PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, 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. This patch makes relevant functions, as well as iterator defition, 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 fd6523c66b..0f82cbe1bd 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -634,25 +634,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
@@ -661,7 +642,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; @@ -693,7 +674,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); @@ -713,7 +694,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; @@ -834,7 +815,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;
@@ -879,19 +860,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.
@@ -907,7 +880,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 0c88b59a4a..577e6b4592 100644 --- a/include/fat.h +++ b/include/fat.h @@ -187,6 +187,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);
If CONFIG_FAT is not set, MAX_CLUSTSIZE is empty. And you get a build error:
CC fs/fs.o In file included from fs/fs.c:12:0: include/fat.h:20:23: error: ‘CONFIG_FS_FAT_MAX_CLUSTSIZE’ undeclared here (not in a function); did you mean ‘CONFIG_SYS_MAX_FLASH_SECT’? #define MAX_CLUSTSIZE CONFIG_FS_FAT_MAX_CLUSTSIZE ^ include/fat.h:214:19: note: in expansion of macro ‘MAX_CLUSTSIZE’ u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); ^~~~~~~~~~~~~ make[1]: *** [scripts/Makefile.build:279: fs/fs.o] Error 1
Thank you for catching this.
We should not assume that every board uses FAT.
The contents of include/fat.h is totally private to fat implementation. So I'd like to guard this include file like:
#ifndef _FAT_H #define _FAT_H
#ifdef CONFIG_FS_FAT
<fat definitions>...
#endif /* CONFIG_FS_FAT */
#endif /* _FAT_H */
-Takahiro AKASHI
Best regards
Heinrich
+} 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);
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 0f82cbe1bd..d9bfb08d97 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -649,6 +649,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; @@ -684,9 +685,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; @@ -704,7 +707,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); @@ -725,18 +728,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; } } @@ -752,8 +756,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; @@ -906,6 +913,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 577e6b4592..bc0f77abb5 100644 --- a/include/fat.h +++ b/include/fat.h @@ -198,6 +198,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 */
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 3b77557b3e..6c715a70f4 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,
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
The patch has to be rebased on current master due to
fs: fat: cannot write to subdirectories 0dc1bfb7302d220a48364263d5632d6d572b069b
which has been merged into U-Boot master.
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 3b77557b3e..6c715a70f4 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[] = "!#$%&'()-.@^`_{}~";
The logic below gets simpler if you define the illegal characters like in the aforementioned patch.
Best regards
Heinrich
- 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;- }
<snip />
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 6c715a70f4..1e4f5af910 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);
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
Some of the error message are written with debug() others with printf().
Shouldn't we switch everything to debug() so that the EFI console is not messed up?
Best regards
Heinrich
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 6c715a70f4..1e4f5af910 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;@@ -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;@@ -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);
On Fri, Jul 20, 2018 at 07:55:06PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
Some of the error message are written with debug() others with printf().
Right, but as you know, there is already a mixture of debug() and printf() in the existing code. Here I tried to do my best to use either one so as to make the usage consistent with existing code though I don't know the exact discipline.
Shouldn't we switch everything to debug() so that the EFI console is not messed up?
Is this a different issue, isn't it? (I suppose that one of Rob's patches tried to address this issue.)
So I'd like to listen to any opinion from other folks, too.
Thanks, -Takahiro AKASHI
Best regards
Heinrich
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 6c715a70f4..1e4f5af910 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;@@ -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;@@ -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);
In this patch, write implementation is overhauled and rewritten by making full use of directory iterator. The ovbious 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 | 469 +++++++++++++++------------------------------ 2 files changed, 157 insertions(+), 321 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index d9bfb08d97..9bafc3a40c 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 1e4f5af910..96edb6674c 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->bs_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,60 @@ 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.bs_total_sect; + if (total_sector == 0) + total_sector = (int)cur_part_info.size; /* cast of lbaint_t */ + + 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 +883,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 +928,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 +951,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; }
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 | 178 +++++++++++++++++---------------------------- 1 file changed, 65 insertions(+), 113 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 96edb6674c..3a9c53e253 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -528,6 +528,43 @@ 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 +572,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, +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 +612,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 +659,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 +677,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 +794,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 +805,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; @@ -841,47 +852,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 */
@@ -909,32 +881,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; @@ -973,6 +926,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); }
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 maitain compatibility with the current interface.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- fs/fat/fat_write.c | 287 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 272 insertions(+), 15 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 3a9c53e253..cc45a33876 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -450,6 +450,120 @@ 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 at 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 */ @@ -579,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; @@ -852,6 +1098,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 { @@ -872,6 +1128,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 */ @@ -921,10 +1183,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); }
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 maitain compatibility with the current interface.
This does not match the EFI spec.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
When testing I observed that I could not write at an offset larger than the file size. This does not match the EFI spec:
EFI_FILE_PROTOCOL.SetPosition(): seeking past the end of the file is allowed (a subsequent write would grow the file).
Best regards
Heinrich
fs/fat/fat_write.c | 287 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 272 insertions(+), 15 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 3a9c53e253..cc45a33876 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -450,6 +450,120 @@ 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 at 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
*/ @@ -579,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;elseoffset = 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;
@@ -852,6 +1098,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 {
@@ -872,6 +1128,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 */
@@ -921,10 +1183,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);
}
On Fri, Jul 20, 2018 at 07:46:49PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 maitain compatibility with the current interface.
This does not match the EFI spec.
First, please understand my standpoint that I mentioned in a reply to your comment on my cover letter.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
When testing I observed that I could not write at an offset larger than the file size. This does not match the EFI spec:
So this is an intended behavior.
EFI_FILE_PROTOCOL.SetPosition(): seeking past the end of the file is allowed (a subsequent write would grow the file).
Surely, this is a discussion point. Any comment from other folks?
-Takahiro AKASHI
Best regards
Heinrich
fs/fat/fat_write.c | 287 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 272 insertions(+), 15 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 3a9c53e253..cc45a33876 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -450,6 +450,120 @@ 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 at 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
*/ @@ -579,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;elseoffset = 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;
@@ -852,6 +1098,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 {
@@ -872,6 +1128,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 */
@@ -921,10 +1183,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);
}
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 03de5d11af..2a5f7bfc26 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'" );
"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 33808d549e..3cb6b21fe9 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 163da103b4..fbaee154dd 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
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
"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 33808d549e..3cb6b21fe9 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,
Why should we create a dummy function? Just use NULL as an indicator that the interface method is not implemented. See the use of structures for the driver model.
@Simon The array fstypes[] should be replaced by a linker list to match the driver model.
}, #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);
Please, check if mkdir is NULL before calling.
- fs_type = FS_TYPE_ANY;
- fs_close();
What do you want to close here if mkdir() is not implemented by the file system?
Best regards
Heinrich
- 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 163da103b4..fbaee154dd 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
Hi,
On 20 July 2018 at 11:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
"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(+)
We need to get a proper fs test in place before we add any more stuff.
Does someone waent to try converting fs-test.sh to pytest in test/py/tests ?
Regards, Simon
On Mon, Jul 23, 2018 at 05:48:13PM -0600, Simon Glass wrote:
Hi,
On 20 July 2018 at 11:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
"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(+)
We need to get a proper fs test in place before we add any more stuff.
Agreed that we need more tests as part of this series.
Does someone waent to try converting fs-test.sh to pytest in test/py/tests ?
I thought there was at least a first pass at that?
On Mon, Jul 23, 2018 at 07:56:58PM -0400, Tom Rini wrote:
On Mon, Jul 23, 2018 at 05:48:13PM -0600, Simon Glass wrote:
Hi,
On 20 July 2018 at 11:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
"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(+)
We need to get a proper fs test in place before we add any more stuff.
Agreed that we need more tests as part of this series.
Is this a new standard rule for new features? Just kidding, but testing was definitely one of my concerns partly because fs-test.sh is anyhow in an old fashion and partly because I can't find any criteria for test coverage: - white-box test or black-box test - coverage for corner (or edge) cases - some sort of stress test, etc.
Does someone waent to try converting fs-test.sh to pytest in test/py/tests ?
I thought there was at least a first pass at that?
Well, I don't see any file system related scripts under test/py/tests.
Please advise me.
Thanks, -Takahiro AKASHI
-- Tom
Hi,
On 23 July 2018 at 19:45, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
On Mon, Jul 23, 2018 at 07:56:58PM -0400, Tom Rini wrote:
On Mon, Jul 23, 2018 at 05:48:13PM -0600, Simon Glass wrote:
Hi,
On 20 July 2018 at 11:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
"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(+)
We need to get a proper fs test in place before we add any more stuff.
Agreed that we need more tests as part of this series.
Is this a new standard rule for new features? Just kidding, but testing was definitely one of my concerns partly because fs-test.sh is anyhow in an old fashion and partly because I can't find any criteria for test coverage:
- white-box test or black-box test
- coverage for corner (or edge) cases
- some sort of stress test, etc.
Does someone waent to try converting fs-test.sh to pytest in test/py/tests ?
I thought there was at least a first pass at that?
Well, I don't see any file system related scripts under test/py/tests.
Please advise me.
It should be possible to translate it from shell to Python without too much trouble if you able able to give it a crack. See for example:
8729d58259 test: Convert the vboot test to test/py
Re testing in general, my view is:
- Unit tests / smaller tests are better than large ones - We don't need stress tests for functionality (e.g. the current FS test uses filesystems that are quite large, and I think they could just be very very small and still get good test coverage and run faster) - Corner cases are an important part of tests (and easy to test with unit tests) - Use the pytest framework
One day we'll enable code coverage for U-Boot sandbox and get a feel for the missing test coverage.
Regards, Simon
On Fri, Jul 20, 2018 at 07:35:18PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
"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 33808d549e..3cb6b21fe9 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,Why should we create a dummy function? Just use NULL as an indicator
because I followed coding style of the existing code here.
that the interface method is not implemented. See the use of structures for the driver model.
Can any file system ever be based on driver model?
@Simon The array fstypes[] should be replaced by a linker list to match the driver model.
}, #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);
Please, check if mkdir is NULL before calling.
No other place checks for NULL. Is this really required?
- fs_type = FS_TYPE_ANY;
- fs_close();
What do you want to close here if mkdir() is not implemented by the file system?
I'm not quite confident here but ->close() be paired with ->probe(), which is set to be called in fs_set_blk_dev().
Thanks, -Takahiro AKASHI
Best regards
Heinrich
- 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 163da103b4..fbaee154dd 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
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 9bafc3a40c..ade5264551 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -639,6 +639,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; @@ -674,6 +675,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 bc0f77abb5..295da0f243 100644 --- a/include/fat.h +++ b/include/fat.h @@ -197,6 +197,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 */
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 | 138 +++++++++++++++++++++++++++++++++++++++++++++ fs/fs.c | 3 +- include/fat.h | 1 + 3 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index cc45a33876..781883c9f4 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1185,3 +1185,141 @@ 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.bs_total_sect; + if (total_sector == 0) + total_sector = (int)cur_part_info.size; /* cast of lbaint_t */ + + 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 3cb6b21fe9..a92e060296 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 295da0f243..039b6e03de 100644 --- a/include/fat.h +++ b/include/fat.h @@ -237,5 +237,6 @@ 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 /* _FAT_H_ */
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
The patch does set the creation date of the directory according to the real time clock but to 1980-01-01T00:00:00Z.
$ ls /mnt/dir1/ -la --full-time drwxr-xr-x 2 root root 2048 1980-01-01 01:00:00.000000000 +0100 dir3
Please, set the time correctly if the real time clock is available.
Best regards
Heinrich
fs/fat/fat_write.c | 138 +++++++++++++++++++++++++++++++++++++++++++++ fs/fs.c | 3 +- include/fat.h | 1 + 3 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index cc45a33876..781883c9f4 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1185,3 +1185,141 @@ 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.bs_total_sect;
- if (total_sector == 0)
total_sector = (int)cur_part_info.size; /* cast of lbaint_t */- 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 3cb6b21fe9..a92e060296 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 295da0f243..039b6e03de 100644 --- a/include/fat.h +++ b/include/fat.h @@ -237,5 +237,6 @@ 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 /* _FAT_H_ */
On Fri, Jul 20, 2018 at 07:14:21PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
The patch does set the creation date of the directory according to the real time clock but to 1980-01-01T00:00:00Z.
$ ls /mnt/dir1/ -la --full-time drwxr-xr-x 2 root root 2048 1980-01-01 01:00:00.000000000 +0100 dir3
Please, set the time correctly if the real time clock is available.
Good point, but I'd like to put this issue into future TODO list as it will end up introducing a new API, such as gettimeofday()?
(and this is not a FAT-specific issue.)
Thanks, -Takahiro AKASHI
Best regards
Heinrich
fs/fat/fat_write.c | 138 +++++++++++++++++++++++++++++++++++++++++++++ fs/fs.c | 3 +- include/fat.h | 1 + 3 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index cc45a33876..781883c9f4 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -1185,3 +1185,141 @@ 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.bs_total_sect;
- if (total_sector == 0)
total_sector = (int)cur_part_info.size; /* cast of lbaint_t */- 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 3cb6b21fe9..a92e060296 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 295da0f243..039b6e03de 100644 --- a/include/fat.h +++ b/include/fat.h @@ -237,5 +237,6 @@ 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 /* _FAT_H_ */
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 2a5f7bfc26..136a5114c6 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
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
Please, provide a Python test with an additional patch.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
On Fri, Jul 20, 2018 at 07:09:17PM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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
Please, provide a Python test with an additional patch.
Please see my reply to Simon's/Tom's comments.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
Thanks a lot for your review. -Takahiro AKASHI
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 e6a15bcb52..6ec98c8022 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); }
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 6ec98c8022..12044a0c71 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,
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 2e8d5ee4df..7b0c5ea56f 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
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 2e8d5ee4df..7b0c5ea56f 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
Tom suggested in https://lists.denx.de/pipermail/u-boot/2018-July/336378.html that the comment headers in test/fs/fs-test.sh should be updated to reflect the total number of passes and fails.
Best regards
Heinrich
On Sun, Jul 29, 2018 at 09:02:00AM +0200, Heinrich Schuchardt wrote:
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
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 2e8d5ee4df..7b0c5ea56f 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
Tom suggested in https://lists.denx.de/pipermail/u-boot/2018-July/336378.html that the comment headers in test/fs/fs-test.sh should be updated to reflect the total number of passes and fails.
Thank you for this heads-up. I'm now trying to rewrite fs-test.sh (at least part of it) on top of py.test framework. It will take some time as I'm still learning how to write tests for py.test.
-Takahiro AKASHI
Best regards
Heinrich
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
This patch series 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
Patch#1 to patch#6 are some sort of preparatory ones. Patch#7 implements write with sub-directories path. Patch#8 to patch#10 implement write with non-zero offset. Patch#11 to patch#15 are related to creating a directory. Patch#16 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. Finally, patch#17 fixes a minor bug in fs-test.sh.
I applied this patch set on top of v2018.07 along with a couple of yet-to--be-upstreamed UEFI-related patches, and could successfully run unmodified SCT[1] on qemu-arm.
Thanks Takahiro for getting this all implemented.
I have some questions concerning code pages:
The FAT file system contains a short name and a long name. The short name is in the 8-bit code page of the system. The long name is 16bit Unicode.
Which local code page do you assume? I would suggest to use 437 as we do in some other drivers.
With your patch fat_mkdir() and fat_file_write() do not yet support UTF-8 as file names and convert this to the 437 local page for the short name and to UTF-16 for the long name.
Without this conversoin we will not be able to implement the EFI specification.
normalize_longname() simply ignores codes 0x80-0xFF. Please, have a look at
https://www.win.tue.nl/~aeb/linux/fs/fat/fatgen103.pdf Microsoft Extensible Firmware Initiative FAT32 File System Specification FAT: General Overview of On-Disk Format Version 1.03, December 6, 2000
Shouldn't we implement the base-name algorithm as described in the paper?
Best regards
Heinrich
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Best regards
Heinrich
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote:
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
I absolutely agree. To be clear, the aim of my FAT write patchset is to successfully run SCT to evaluate and verify UEFI-related features rather than to make the implementation comply with UEFI specification.
For the latter purpose, separate patches would be compiled as it might require time-consuming efforts. (I'm sure that SCT's file (media access) protocol test causes lots of test case failures.)
Thanks, -Takahiro AKASHI
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Best regards
Heinrich
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote:
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Yes, a follow-up series makes sense, and yes, efi-next for the patches themselves sounds fine, thanks!
On 07/23/2018 04:46 PM, Tom Rini wrote:
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote:
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Yes, a follow-up series makes sense, and yes, efi-next for the patches themselves sounds fine, thanks!
Hello Takahiro,
could you, please, resubmit the series with the few changes needed so that Alex can consider them for efi-next.
Cheers
Heinrich
On Tue, Aug 07, 2018 at 12:34:28AM +0200, Heinrich Schuchardt wrote:
On 07/23/2018 04:46 PM, Tom Rini wrote:
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote:
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Yes, a follow-up series makes sense, and yes, efi-next for the patches themselves sounds fine, thanks!
Hello Takahiro,
could you, please, resubmit the series with the few changes needed so that Alex can consider them for efi-next.
Well, I'm sure that my patch set can be applied to v2018.09-rc1 w/o any changes if your patch (0dc1bfb7302 "fs: fat: cannot write to subdirectories") is reverted beforehand. As I said, my patch(#5/17) superseded it in terms of its function.
What's more, I ran fs-test.sh again, and all tests passed, either against fat16 or fat32. Do we still want to update a test result in fs-test.sh? (removing it would be the easiest.)
Thanks, -Takahiro AKASHI
Cheers
Heinrich
On Tue, Aug 07, 2018 at 02:38:34PM +0900, AKASHI Takahiro wrote:
On Tue, Aug 07, 2018 at 12:34:28AM +0200, Heinrich Schuchardt wrote:
On 07/23/2018 04:46 PM, Tom Rini wrote:
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote:
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Yes, a follow-up series makes sense, and yes, efi-next for the patches themselves sounds fine, thanks!
Hello Takahiro,
could you, please, resubmit the series with the few changes needed so that Alex can consider them for efi-next.
Well, I'm sure that my patch set can be applied to v2018.09-rc1 w/o any changes if your patch (0dc1bfb7302 "fs: fat: cannot write to subdirectories") is reverted beforehand. As I said, my patch(#5/17) superseded it in terms of its function.
See: https://git.linaro.org/people/takahiro.akashi/u-boot.git fat_write
What's more, I ran fs-test.sh again, and all tests passed, either against fat16 or fat32. Do we still want to update a test result in fs-test.sh? (removing it would be the easiest.)
I've almost finished to convert fs-test.sh to a Python script, but it will take some time to add more tests against my patch set as my priority right now is on examining a test result from SCT.
-Takahiro AKASHI
Thanks, -Takahiro AKASHI
Cheers
Heinrich
On 08/07/2018 07:38 AM, AKASHI Takahiro wrote:
On Tue, Aug 07, 2018 at 12:34:28AM +0200, Heinrich Schuchardt wrote:
On 07/23/2018 04:46 PM, Tom Rini wrote:
On Sun, Jul 22, 2018 at 08:44:39AM +0200, Heinrich Schuchardt wrote:
Hello Tom, hello Alex,
I have been testing the patches. They are working fine for ASCII file names. To support Unicode file names extra work will be needed. But probably we should postpone this to a later patch series.
There are some dependencies with my work for correcting errors in Unicode handling for the EFI branch. Should the patches be passed via efi-next?
Yes, a follow-up series makes sense, and yes, efi-next for the patches themselves sounds fine, thanks!
Hello Takahiro,
could you, please, resubmit the series with the few changes needed so that Alex can consider them for efi-next.
Well, I'm sure that my patch set can be applied to v2018.09-rc1 w/o any changes if your patch (0dc1bfb7302 "fs: fat: cannot write to subdirectories") is reverted beforehand. As I said, my patch(#5/17) superseded it in terms of its function.
Please, add a patch doing the reverting or rebase patch 5.
What's more, I ran fs-test.sh again, and all tests passed, either against fat16 or fat32. Do we still want to update a test result in fs-test.sh? (removing it would be the easiest.)
As long as we have no better tests in place updating those comments is appropriate.
In my review of patch 01/17 I indicated an error in evaluating the sector number.
Best regards
Heinrich
participants (4)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Simon Glass -
Tom Rini