FAT: Add FAT write feature
PATCH v2 1/1 FAT: Add FAT write feature
In some cases, saving data in RAM as a file with FAT format is required. This patch allows the file to be written in FAT formatted partition.
The usage is similar with reading a file.
First, fat_register_device function is called before file_fat_write function in order to set target partition.
Then, file_fat_write function is invoked with desired file name, start ram address for writing data, and file size.
---
Changes for v2:
- update README file
README | 5 + fs/fat/Makefile | 1 + fs/fat/fat.c | 2 + fs/fat/fat_write.c | 1091 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fat.h | 3 + 5 files changed, 1102 insertions(+), 0 deletions(-) create mode 100644 fs/fat/fat_write.c
README +5 -0
@@ -1144,6 +1144,11 @@ The following options need to be configured: to disable the command chpart. This is the default when you have not defined a custom partition +- FAT(File Allocation Table) filesystem write function support: + CONFIG_FAT_WRITE + Support for saving memory data as a file + in FAT formatted partition + - Keyboard Support: CONFIG_ISA_KEYBOARD
fs/fat/Makefile +1 -0
@@ -25,6 +25,7 @@ LIB = $(obj)libfat.o AOBJS = COBJS-$(CONFIG_CMD_FAT) := fat.o +COBJS-$(CONFIG_FAT_WRITE):= fat_write.o ifndef CONFIG_SPL_BUILD COBJS-$(CONFIG_CMD_FAT) += file.o
fs/fat/fat.c +2 -0
@@ -46,6 +46,7 @@ static void downcase (char *str)static block_dev_desc_t *cur_dev =NULL ;static unsigned long part_offset =0 ; +static unsigned long part_size;static int cur_part =1 ;
@@ -99,6 +100,7 @@ int fat_register_device (block_dev_desc_t * dev_desc, int part_no)if (!get_partition_info (dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; + part_size = info.size; }else if ((strncmp ((char *)&buffer[DOS_FS_TYPE_OFFSET ],"FAT" ,3 ) ==0 ) || (strncmp ((char *)&buffer[DOS_FS32_TYPE_OFFSET ],"FAT32" ,5 ) ==0 )) {/* ok, we assume we are on a PBR only */
fs/fat/fat_write.c new file +1091 -0
1,091 lines not on this page · raw message
include/fat.h +3 -0
@@ -99,6 +99,8 @@#endif #define TOLOWER (c) if((c) >= 'A' && (c) <= 'Z'){(c)+=('a' - 'A');} +#define TOUPPER (c) if ((c) >= 'a' && (c) <= 'z') \ + (c) -= ('a' - 'A');#define START (dent) (FAT2CPU16((dent)->start) \ + (mydata->fatsize != 32 ? 0 : \ (FAT2CPU16((dent)->starthi) << 16)))
@@ -210,4 +212,5 @@ long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);const char *file_getfsname (int idx);int fat_register_device (block_dev_desc_t *dev_desc,int part_no); +int file_fat_write (const char *filename,void *buffer,unsigned long maxsize);#endif /* _FAT_H_ */
Signature · 1 line
1.7.4.1
Wolfgang Denk
AppliedPATCH v2 1/1 FAT: Add FAT write feature
Dear Donggeun Kim,
Quoted 15 lines · In message <[email protected]> you wrote:
In some cases, saving data in RAM as a file with FAT format is required. This patch allows the file to be written in FAT formatted partition.
The usage is similar with reading a file.
First,fat_register_devicefunction is called beforefile_fat_writefunction in order to set target partition.
Then,file_fat_writefunction is invoked with desired file name, start ram address for writing data, and file size.Signed-off-by Donggeun Kim <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes for v2:
- update README file
Quoted 7 lines
README | 5 + fs/fat/Makefile | 1 + fs/fat/fat.c | 2 + fs/fat/fat_write.c | 1091 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fat.h | 3 + 5 files changed, 1102 insertions(+), 0 deletions(-) create mode 100644 fs/fat/fat_write.c
Applied, thanks.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Celestial navigation is based on the premise that the Earth is the center of the universe. The premise is wrong, but the navigation works. An incorrect model can be a useful tool. - Kelvin Throop III
PATCH v2 1/1 FAT: Add FAT write feature
Quoted 28 lines
Dear Donggeun Kim,
Quoted 16 lines · In message <[email protected]> you wrote:
In some cases, saving data in RAM as a file with FAT format is required. This patch allows the file to be written in FAT formatted partition.
The usage is similar with reading a file.
First,fat_register_devicefunction is called beforefile_fat_writefunction in order to set target partition.
Then,file_fat_writefunction is invoked with desired file name, start ram address for writing data, and file size.Signed-off-by Donggeun Kim <[email protected]>Signed-off-by Kyungmin Park <[email protected]>---
Changes for v2:
- update README fileQuoted 8 lines
README | 5 + fs/fat/Makefile | 1 + fs/fat/fat.c | 2 + fs/fat/fat_write.c | 1091 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fat.h | 3 + 5 files changed, 1102 insertions(+), 0 deletions(-) create mode 100644 fs/fat/fat_write.c
Quoted 1 line
Applied, thanks.
DAMN :-(
Dongguen,
I now get this warning on many boards:
fat.c:49:22: warning: 'part_size' defined but not used
please FIX !
Also, can you avoid using global variables, it's stupid?
Cheers
PATCH v2 1/1 FAT: Add FAT write feature
In reply to Donggeun KimQuoted 2 lines · Donggeun Kim
In some cases, saving data in RAM as a file with FAT format is required. This patch allows the file to be written in FAT formatted partition.
i thought Wolfgang NAK-ed FS write patches in the past ...
Quoted 5 lines
fs/fat/Makefile +0
-0?COBJS-$(CONFIG_CMD_FAT) ? ? ? ?:= fat.o
+COBJS-$(CONFIG_FAT_WRITE):= fat_write.o
missing whitespace before that ":="
Quoted 16 lines
fs/fat/fat_write.c new file +13
-0+/* +* fat_write.c +* +* R/W (V)FAT 12/16/32 filesystem implementation by Donggeun Kim +* +* See file CREDITS for list of people who contributed to this +* project. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 of +* the License, or (at your option) any later version. +*
fs/fat/fat_write.c line 23
Quoted 10 lines
fs/fat/fat_write.c new file +10
-0+* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, +* MA 02111-1307 USA +*/
there's no actual copyright here. you wrote this all from scratch ?
fs/fat/fat_write.c
Quoted 5 lines
fs/fat/fat_write.c new file +5
-0+static void set_name (dir_entry *dirent,const char *filename) +{ + ? ? ? int period_location, len, i, ext_num; + + ? ? ? len = strlen(filename);
i/period_location/len should be size_t, not int
Quoted 2 lines
+ ? ? ?
memcpy(s_name, filename, len);
+ ? ? ?uppercase(s_name, len);
you use this uppercase() func exactly once, and do a memcpy right before. drop the memcpy, and inline the uppercase func and use the proper toupper helper from linux/ctype.h
Quoted 4 lines
+ ? ? ? /* Pad spaces when the length of file name is shorter than eight */
+ ? ? ? if (period_location< 8) {
+ ? ? ? ? ? ? ?memcpy(dirent->name, s_name, period_location);
+ ? ? ? ? ? ? ? for (i =period_location; i < 8; i++)
Quoted 8 lines
+ ? ? ? ? ? ? ? ? ? ? ? dirent->name[i] = ' ';
+ ? ? ? } else if (period_location== 8) {
+ ? ? ? ? ? ? ?memcpy(dirent->name, s_name, period_location);
+ ? ? ? } else {
+ ? ? ? ? ? ? ?memcpy(dirent->name, s_name, 6);
+ ? ? ? ? ? ? ? dirent->name[6] = '~';
+ ? ? ? ? ? ? ? dirent->name[7] = '1';
+ ? ? ? }
err, doesn't this "~1" hardcode assume that there is no "~1" file already existing ? and if there happens to be, you blow it away/corrupt the fs ?
fs/fat/fat_write.c
Quoted 9 lines
fs/fat/fat_write.c new file +9
-0+static __u32 get_fatent_value (fsdata *mydata,__u32 entry) +{ + ? ? ? /* Get the actual entry from the table */ + ? ? ? switch (mydata->fatsize) { + ? ? ? case 32: + ? ? ? ? ? ? ? ret = FAT2CPU32(((__u32 *) mydata->fatbuf)[offset]); + ? ? ? ? ? ? ? break; + ? ? ? case 16: + ? ? ? ? ? ? ? ret = FAT2CPU16(((__u16 *) mydata->fatbuf)[offset]);
ugh, whoever dreamt up these FAT2CPU* macros should get smacked. these need to get scrubbed from fat.h and all code in u-boot and replaced with proper get/put unaligned macros from include/linux/.
fs/fat/fat_write.c line 316
Quoted 2 lines
fs/fat/fat_write.c new file +2
-0+static int is_next_clust (fsdata *mydata,dir_entry *dentptr); +static void flush_dir_table (fsdata *mydata,dir_entry **dentptr);
re-order these tiny funcs in the file to avoid having to use prototypes
Quoted 36 lines
fs/fat/fat_write.c new file +36
-0+/* +* Fill dir_slot entries with appropriate name, id, and attr +* The real directory entry is returned by 'dentptr' +*/ +static void +fill_dir_slot (fsdata *mydata,dir_entry **dentptr,const char *l_name) +{ + ? ? ? dir_slot *slotptr = (dir_slot *)get_vfatname_block; + ? ? ? __u8 counter, checksum; + ? ? ? int idx = 0, ret; + ? ? ? char s_name[16]; + + ? ? ? /* Get short file name and checksum value */ + ? ? ? strncpy(s_name, (*dentptr)->name, 16); + ? ? ? checksum = mkcksum(s_name); + + ? ? ? do { + ? ? ? ? ? ? ? memset(slotptr, 0x00, sizeof(dir_slot)); + ? ? ? ? ? ? ? ret = str2slot(slotptr, l_name, &idx); + ? ? ? ? ? ? ? slotptr->id = ++counter; + ? ? ? ? ? ? ? slotptr->attr = ATTR_VFAT; + ? ? ? ? ? ? ? slotptr->alias_checksum = checksum; + ? ? ? ? ? ? ? slotptr++; + ? ? ? } while (ret == 0); + + ? ? ? slotptr--; + ? ? ? 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)++; + ? ? ? ? ? ? ? slotptr--;
fs/fat/fat_write.c
Quoted 10 lines
fs/fat/fat_write.c new file +10
-0+ ? ? ? ? ? ? ? counter--; + ? ? ? } + + ? ? ? if (is_next_clust(mydata, *dentptr)) { + ? ? ? ? ? ? ? /* A new cluster is allocated for directory table */ + ? ? ? ? ? ? ? flush_dir_table(mydata, dentptr); + ? ? ? } +} + +static __u32 dir_curclust;
all these global variables make me cry. i guess it's a good thing u-boot is single threaded.
fs/fat/fat_write.c
Quoted 10 lines
fs/fat/fat_write.c new file +9
-0+static int set_fatent_value (fsdata *mydata,__u32 entry,__u32 entry_value) +{ + ? ? ? /* Set the actual entry */ + ? ? ? switch (mydata->fatsize) { + ? ? ? case 32: + ? ? ? ? ? ? ? ((__u32 *) mydata->fatbuf)[offset] = cpu_to_le32(entry_value); + ? ? ? ? ? ? ? break; + ? ? ? case 16: + ? ? ? ? ? ? ? ((__u16 *) mydata->fatbuf)[offset] = cpu_to_le16(entry_value);
use proper put_unaligned helpers from include/linux/
fs/fat/fat_write.c
Quoted 4 lines
fs/fat/fat_write.c new file +3
-0+static int is_next_clust (fsdata *mydata,dir_entry *dentptr) +{ + ? ? ? cur_position = (__u8 *)dentptr - get_dentfromdir_block;
you've got a lot of random casts throughout this file. makes me really wonder as to its sanity.
Quoted 1 line
+ ? ? ? ? ? ? ? ? ? ? ?
printf("Error: flush fat buffer\n");
all of these printf() calls without any format args should be puts() instead
include/fat.h line 103
Quoted 6 lines
include/fat.h +2
-0#define TOLOWER (c) if((c) >= 'A' && (c) <= 'Z'){(c)+=('a' - 'A');} +#define TOUPPER (c) if ((c) >= 'a' && (c) <= 'z') \ + (c) -= ('a' - 'A');
why do these exist ? we already have tolower/toupper in linux/ctype.h, and macros shouldn't be embedding ugly if/assignments. -mike
PATCH v2 1/1 FAT: Add FAT write feature
Dear Mike Frysinger,
Quoted 5 lines · In message <CAJaTeToXsLLiqKb1NZpi457OW39admnPtWSrRZVkpRPNXJ6wiA@mail.gmail.com>…
Quoted 2 lines · Donggeun Kim
In some cases, saving data in RAM as a file with FAT format is required. This patch allows the file to be written in FAT formatted partition.
i thought Wolfgang NAK-ed FS write patches in the past ...
If I did, then for technical issues that needed to be cleaned up. I think write support is a good thing to have.
Quoted 4 lines
Quoted 2 lines
?COBJS-$(CONFIG_CMD_FAT) ? ? ? ?:= fat.o
+COBJS-$(CONFIG_FAT_WRITE):= fat_write.omissing whitespace before that ":="
Argh.. THe patch has already been applied, so this needs to be cleaned up in a separate patch now.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Uncontrolled power will turn even saints into savages. And we can all be counted on to live down to our lowest impulses. -- Parmen, "Plato's Stepchildren", stardate 5784.3
PATCH v2 1/1 FAT: Add FAT write feature
Quoted 9 lines · Wolfgang Denk
Quoted 5 lines · Mike Frysinger
Quoted 2 lines · Donggeun Kim
In some cases, saving data in RAM as a file with FAT format is required. This patch allows the file to be written in FAT formatted partition.
i thought Wolfgang NAK-ed FS write patches in the past ...
If I did, then for technical issues that needed to be cleaned up. I think write support is a good thing to have.
np. i thought i recalled old patches/proposals like being able to mv on a jffs2 being NAK-ed on the grounds that if you want to modify a FS, you should boot into a real OS. maybe i'm just making this all up in my head though.
-mike
PATCH 1/1 fs/fat/fat.c: fix warning: 'part_size' defined but not used
In reply to Donggeun KimCommit c30a15e "FAT: Add FAT write feature" introduced a compiler warning. Fix this.
---
fs/fat/fat.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
fs/fat/fat.c +0 -2
@@ -46,7 +46,6 @@ static void downcase (char *str)static block_dev_desc_t *cur_dev =NULL ;static unsigned long part_offset =0 ; -static unsigned long part_size;static int cur_part =1 ;
@@ -100,7 +99,6 @@ int fat_register_device (block_dev_desc_t * dev_desc, int part_no)if (!get_partition_info (dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; - part_size = info.size; }else if ((strncmp ((char *)&buffer[DOS_FS_TYPE_OFFSET ],"FAT" ,3 ) ==0 ) || (strncmp ((char *)&buffer[DOS_FS32_TYPE_OFFSET ],"FAT32" ,5 ) ==0 )) {/* ok, we assume we are on a PBR only */
Signature · 1 line
1.7.6.4
PATCH 1/1 fs/fat/fat.c: fix warning: 'part_size' defined but not used
Quoted 29 lines · Wolfgang Denk
Commit c30a15e "FAT: Add FAT write feature" introduced a compiler warning. Fix this.
Signed-off-by Wolfgang Denk <[email protected]>Cc Donggeun Kim <[email protected]>Cc Kyungmin Park <[email protected]>---
fs/fat/fat.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)fs/fat/fat.c +0
-2@@ -46,7 +46,6 @@ static void downcase (char *str)static block_dev_desc_t *cur_dev =NULL ;static unsigned long part_offset =0 ; -static unsigned long part_size;static int cur_part =1 ;@@ -100,7 +99,6 @@ int fat_register_device (block_dev_desc_t * dev_desc, int part_no) if (!get_partition_info(dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; - part_size = info.size; } else if ((strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3) == 0)
Quoted 2 lines
|| (
strncmp((char *)&buffer[DOS_FS32_TYPE_OFFSET], "FAT32", 5)== 0)) { /* ok, we assume we are on a PBR only */
Hi Wolfgang,
I know it's rather late to comment on this, but this patch breaks FAT write support.
-Aaron
Signature · 2 lines
Aaron Williams <[email protected]> (408) 943-7198
PATCH 1/1 fs/fat/fat.c: fix warning: 'part_size' defined but not used
Dear Aaron Williams,
Quoted 3 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Commit c30a15e "FAT: Add FAT write feature" introduced a compiler warning. Fix this.
...
Quoted 2 lines
I know it's rather late to comment on this, but this patch breaks FAT write support.
How can it break something we don't have?
Currently there is no write support for (V)FAT file systems in mainline.
The commit removes a static variable of file scope, that was used in a single place, where a value was assigned to it. There was no place anywhere in the code twhere else this variable was referenced. So how can this break anything?
Please elucidate.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de For every complex problem, there is a solution that is simple, neat, and wrong. - Mark Twain
PATCH 1/1 fs/fat/fat.c: fix warning: 'part_size' defined but not used
Hello all,
Quoted 15 lines · On Tue, 13 Dec 2011 09:18:15 +0100 Wolfgang Denk <[email protected]> wrote:
Dear Aaron Williams,
Quoted 3 lines · In message <[email protected]> you wrote:
Quoted 3 lines
Commit c30a15e "FAT: Add FAT write feature" introduced a compiler warning. Fix this.
...
Quoted 2 lines
I know it's rather late to comment on this, but this patch breaks FAT write support.
How can it break something we don't have?
Currently there is no write support for (V)FAT file systems in mainline.
Quoted 6 lines
The commit removes a static variable of file scope, that was used in a single place, where a value was assigned to it. There was no place anywhere in the code twhere else this variable was referenced. So how can this break anything?
Please elucidate.
The FAT write support as submitted and included seems to be broken. The removed variable is referenced in fs/fat/fat_write.c, but in fs/fat/fat.c it is declared as static. This issue didn't show up because no board config file in mainline defines CONFIG_FAT_WRITE.
@ Donggeun Kim
Could you please submit a patch fixing this?
Thanks,
Anatolij
PATCH 1/1 fs/fat/fat.c: fix warning: 'part_size' defined but not used
Dear Anatolij Gustschin,
Quoted 5 lines · In message <20111213095358.5601b9ce@wker> you wrote:
The FAT write support as submitted and included seems to be broken. The removed variable is referenced in
fs/fat/fat_write.c, but infs/fat/fat.cit is declared as static. This issue didn't show up because no board config file in mainline definesCONFIG_FAT_WRITE.
It's not only that. It appears that this code is also not hooked upo to any command a user could call. At least, I cannot see anything like that in common/cmd_fat.c or so...
Quoted 2 lines
@ Donggeun Kim
Could you please submit a patch fixing this?
I guess this needs more fixing...
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "If you own a machine, you are in turn owned by it, and spend your time serving it..." - Marion Zimmer Bradley, _The Forbidden Tower_
PATCH 1/1 fs/fat/fat.c: fix warning: 'part_size' defined but not used
In reply to Aaron WilliamsQuoted 35 lines · Aaron Williams
Quoted 29 lines · Wolfgang Denk
Commit c30a15e "FAT: Add FAT write feature" introduced a compiler warning. Fix this.
Signed-off-by Wolfgang Denk <[email protected]>Cc Donggeun Kim <[email protected]>Cc Kyungmin Park <[email protected]>---
fs/fat/fat.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)fs/fat/fat.c +0
-2@@ -46,7 +46,6 @@ static void downcase (char *str)static block_dev_desc_t *cur_dev =NULL ;static unsigned long part_offset =0 ; -static unsigned long part_size;static int cur_part =1 ;@@ -100,7 +99,6 @@ int fat_register_device (block_dev_desc_t * dev_desc, int part_no) if (!get_partition_info(dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; - part_size = info.size; } else if ((strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3) == 0)Quoted 2 lines
|| (
strncmp((char *)&buffer[DOS_FS32_TYPE_OFFSET], "FAT32", 5)== 0)) { /* ok, we assume we are on a PBR only */Hi Wolfgang,
Quoted 6 lines
I know it's rather late to comment on this, but this patch breaks FAT write support.
-Aaron
Hi, Wolfgang,
I've overlooked the patch before.
If 'part_size' variable is not set in fat_register_device function, FAT write feature would not work properly.
So, I will send a patch to define the variable only when FAT_WRITE is enabled. It can avoid compile warning.
Thanks.
-Donggeun