fs: fat: Fix mkcksum() function parameters
PATCH 1/1 fs: fat: Fix mkcksum() function parameters
The mkcksum() function now takes one parameter, the pointer to 11-byte wide character array, which it then operates on.
Currently, the function is wrongly passed (dir_entry)->name, which is only 8-byte wide character array. Though by further inspecting the dir_entry structure, it can be noticed that the name[8] entry is immediatelly followed by ext[3] entry. Thus, name[8] and ext[3] in the dir_entry structure actually work as this 11-byte wide array since they're placed right next to each other by current compiler behavior.
Depending on this is obviously wrong, thus fix this by correctly passing both (dir_entry)->name and (dir_entry)->ext to the mkcksum() function and adjust the function appropriately.
--- fs/fat/fat.c | 20 ++++++++++++-------- fs/fat/fat_write.c | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-)
fs/fat/fat.c +12 -8
@@ -567,15 +567,16 @@ get_vfatname(fsdata *mydata, int curclust, __u8 *cluster, }/* Calculate short name checksum */ -static __u8 mkcksum (const char *str) +static __u8 mkcksum (const char name[8 ],const char ext[3 ]) {int i;__u8 ret =0 ; -for (i =0 ; i <11 ; i++) { - ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + str[i]; - } +for (i =0 ; i <sizeof (name); i++) + ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + name[i]; +for (i =0 ; i <sizeof (ext); i++) + ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + ext[i];return ret; }
@@ -678,7 +679,8 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,return NULL ; }#ifdef CONFIG_SUPPORT_VFAT -if (dols &&mkcksum (dentptr->name) == prevcksum) { +__u8 csum =mkcksum (dentptr->name, dentptr->ext); +if (dols && csum == prevcksum) { prevcksum =0xffff ; dentptr++;continue ;
@@ -946,13 +948,16 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,for (i =0 ; i <DIRENTSPERBLOCK ; i++) {char s_name[14 ], l_name[VFAT_MAXLEN_BYTES ]; +__u8 csum; l_name[0 ] ='\0' ;if (dentptr->name[0 ] ==DELETED_FLAG ) { dentptr++;continue ; } -if ((dentptr->attr &ATTR_VOLUME )) { + + csum =mkcksum (dentptr->name, dentptr->ext); +if (dentptr->attr &ATTR_VOLUME ) {#ifdef CONFIG_SUPPORT_VFAT if ((dentptr->attr &ATTR_VFAT ) ==ATTR_VFAT && (dentptr->name[0 ] &LAST_LONG_ENTRY_MASK )) {
@@ -1015,8 +1020,7 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, goto exit; }#ifdef CONFIG_SUPPORT_VFAT - else if (dols == LS_ROOT && - mkcksum(dentptr->name) == prevcksum) { + else if (dols == LS_ROOT && csum == prevcksum) { prevcksum =0xffff ; dentptr++;continue ;
fs/fat/fat_write.c +1 -1
@@ -335,7 +335,7 @@ fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)/* Get short file name and checksum value */ strncpy (s_name, (*dentptr)->name,16 ); - checksum =mkcksum (s_name); + checksum =mkcksum (*dentptr->name, *dentptr->ext); do { memset(slotptr, 0x00, sizeof(dir_slot));
Signature · 1 line
1.7.10.4
PATCH 1/1 fs: fat: Fix mkcksum() function parameters
Quoted 11 lines · Marek Vasut
The
mkcksum()function now takes one parameter, the pointer to 11-byte wide character array, which it then operates on.Currently, the function is wrongly passed (dir_entry)->name, which is only 8-byte wide character array. Though by further inspecting the
dir_entrystructure, it can be noticed that the name[8] entry is immediatelly followed by ext[3] entry. Thus, name[8] and ext[3] in thedir_entrystructure actually work as this 11-byte wide array since they're placed right next to each other by current compiler behavior.
Quoted 6 lines
Depending on this is obviously wrong, thus fix this by correctly passing both (dir_entry)->name and (dir_entry)->ext to the
mkcksum()function and adjust the function appropriately.Signed-off-by Marek Vasut <[email protected]>Cc Tom Rini <[email protected]>
Building on current next:
Configuring for trats board...
fat_write.c:338:29: error: request for member 'name' in something not a structure or union
fat_write.c:338:45: error: request for member 'ext' in something not a structure or union
Please fix and submit a v2, thanks.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121009/eb4c1ec9/attachment.pgp>
PATCH v2 1/1 fs: fat: Fix mkcksum() function parameters
In reply to Marek VasutThe mkcksum() function now takes one parameter, the pointer to 11-byte wide character array, which it then operates on.
Currently, the function is wrongly passed (dir_entry)->name, which is only 8-byte wide character array. Though by further inspecting the dir_entry structure, it can be noticed that the name[8] entry is immediatelly followed by ext[3] entry. Thus, name[8] and ext[3] in the dir_entry structure actually work as this 11-byte wide array since they're placed right next to each other by current compiler behavior.
Depending on this is obviously wrong, thus fix this by correctly passing both (dir_entry)->name and (dir_entry)->ext to the mkcksum() function and adjust the function appropriately.
--- fs/fat/fat.c | 20 ++++++++++++-------- fs/fat/fat_write.c | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-)
V2: Fix fat write by adding braces
fs/fat/fat.c +12 -8
@@ -567,15 +567,16 @@ get_vfatname(fsdata *mydata, int curclust, __u8 *cluster, }/* Calculate short name checksum */ -static __u8 mkcksum (const char *str) +static __u8 mkcksum (const char name[8 ],const char ext[3 ]) {int i;__u8 ret =0 ; -for (i =0 ; i <11 ; i++) { - ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + str[i]; - } +for (i =0 ; i <sizeof (name); i++) + ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + name[i]; +for (i =0 ; i <sizeof (ext); i++) + ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + ext[i];return ret; }
@@ -678,7 +679,8 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,return NULL ; }#ifdef CONFIG_SUPPORT_VFAT -if (dols &&mkcksum (dentptr->name) == prevcksum) { +__u8 csum =mkcksum (dentptr->name, dentptr->ext); +if (dols && csum == prevcksum) { prevcksum =0xffff ; dentptr++;continue ;
@@ -946,13 +948,16 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,for (i =0 ; i <DIRENTSPERBLOCK ; i++) {char s_name[14 ], l_name[VFAT_MAXLEN_BYTES ]; +__u8 csum; l_name[0 ] ='\0' ;if (dentptr->name[0 ] ==DELETED_FLAG ) { dentptr++;continue ; } -if ((dentptr->attr &ATTR_VOLUME )) { + + csum =mkcksum (dentptr->name, dentptr->ext); +if (dentptr->attr &ATTR_VOLUME ) {#ifdef CONFIG_SUPPORT_VFAT if ((dentptr->attr &ATTR_VFAT ) ==ATTR_VFAT && (dentptr->name[0 ] &LAST_LONG_ENTRY_MASK )) {
@@ -1015,8 +1020,7 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, goto exit; }#ifdef CONFIG_SUPPORT_VFAT - else if (dols == LS_ROOT && - mkcksum(dentptr->name) == prevcksum) { + else if (dols == LS_ROOT && csum == prevcksum) { prevcksum =0xffff ; dentptr++;continue ;
fs/fat/fat_write.c +1 -1
@@ -335,7 +335,7 @@ fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)/* Get short file name and checksum value */ strncpy (s_name, (*dentptr)->name,16 ); - checksum =mkcksum (s_name); + checksum =mkcksum ((*dentptr)->name, (*dentptr)->ext); do { memset(slotptr, 0x00, sizeof(dir_slot));
Signature · 1 line
1.7.10.4
PATCH v2 1/1 fs: fat: Fix mkcksum() function parameters
Quoted 11 lines · Marek Vasut
The
mkcksum()function now takes one parameter, the pointer to 11-byte wide character array, which it then operates on.Currently, the function is wrongly passed (dir_entry)->name, which is only 8-byte wide character array. Though by further inspecting the
dir_entrystructure, it can be noticed that the name[8] entry is immediatelly followed by ext[3] entry. Thus, name[8] and ext[3] in thedir_entrystructure actually work as this 11-byte wide array since they're placed right next to each other by current compiler behavior.
Quoted 6 lines
Depending on this is obviously wrong, thus fix this by correctly passing both (dir_entry)->name and (dir_entry)->ext to the
mkcksum()function and adjust the function appropriately.Signed-off-by Marek Vasut <[email protected]>Cc Tom Rini <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121017/6f314102/attachment.pgp>
PATCH v2 1/1 fs: fat: Fix mkcksum() function parameters
In reply to Marek VasutHi Marek,
This patch is broken. It breaks detecting duplicate filenames. The problem is that you are using sizeof(name) and sizeof(ext). This does not work since this just reports the size of the pointer in gcc 4.7, which according to our compiler guy is the correct behavior. Instead of using sizeof, just use 8 and 3 respectively. Otherwise it is doing a checksum of 4 and 4 characters.
-Aaron
Quoted 100 lines · Marek Vasut
The
mkcksum()function now takes one parameter, the pointer to 11-byte wide character array, which it then operates on.Currently, the function is wrongly passed (dir_entry)->name, which is only 8-byte wide character array. Though by further inspecting the
dir_entrystructure, it can be noticed that the name[8] entry is immediatelly followed by ext[3] entry. Thus, name[8] and ext[3] in thedir_entrystructure actually work as this 11-byte wide array since they're placed right next to each other by current compiler behavior.Depending on this is obviously wrong, thus fix this by correctly passing both (dir_entry)->name and (dir_entry)->ext to the
mkcksum()function and adjust the function appropriately.Signed-off-by Marek Vasut <[email protected]>Cc Tom Rini <[email protected]>--- fs/fat/fat.c | 20 ++++++++++++-------- fs/fat/fat_write.c | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-)V2: Fix fat write by adding braces
fs/fat/fat.c +12
-8@@ -567,15 +567,16 @@ get_vfatname(fsdata *mydata, int curclust, __u8 *cluster, }/* Calculate short name checksum */ -static __u8 mkcksum (const char *str) +static __u8 mkcksum (const char name[8 ],const char ext[3 ]) {int i;__u8 ret =0 ; -for (i =0 ; i <11 ; i++) { - ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + str[i]; - } +for (i =0 ; i <sizeof (name); i++) + ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + name[i]; +for (i =0 ; i <sizeof (ext); i++) + ret = (((ret &1 ) <<7 ) | ((ret &0xfe ) >>1 )) + ext[i];return ret; }@@ -678,7 +679,8 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int startsect,return NULL ; }#ifdef CONFIG_SUPPORT_VFAT -if (dols &&mkcksum (dentptr->name) == prevcksum) { +__u8 csum =mkcksum (dentptr->name, dentptr->ext); +if (dols && csum == prevcksum) { prevcksum =0xffff ; dentptr++;continue ;@@ -946,13 +948,16 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer,for (i =0 ; i <DIRENTSPERBLOCK ; i++) {char s_name[14 ], l_name[VFAT_MAXLEN_BYTES ]; +__u8 csum; l_name[0 ] ='\0' ;if (dentptr->name[0 ] ==DELETED_FLAG ) { dentptr++;continue ; } -if ((dentptr->attr &ATTR_VOLUME )) { + + csum =mkcksum (dentptr->name, dentptr->ext); +if (dentptr->attr &ATTR_VOLUME ) {#ifdef CONFIG_SUPPORT_VFAT if ((dentptr->attr &ATTR_VFAT ) ==ATTR_VFAT && (dentptr->name[0 ] &LAST_LONG_ENTRY_MASK )) {@@ -1015,8 +1020,7 @@ do_fat_read_at(const char *filename, unsigned long pos, void *buffer, goto exit; }#ifdef CONFIG_SUPPORT_VFAT - else if (dols == LS_ROOT && - mkcksum(dentptr->name) == prevcksum) { + else if (dols == LS_ROOT && csum == prevcksum) { prevcksum =0xffff ; dentptr++;continue ;fs/fat/fat_write.c +1
-1@@ -335,7 +335,7 @@ fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)/* Get short file name and checksum value */ strncpy (s_name, (*dentptr)->name,16 ); - checksum =mkcksum (s_name); + checksum =mkcksum ((*dentptr)->name, (*dentptr)->ext); do { memset(slotptr, 0x00, sizeof(dir_slot));
Signature · 4 lines
Aaron Williams Software Engineer Cavium, Inc. (408) 943-7198 (510) 789-8988 (cell)
PATCH v2 1/1 fs: fat: Fix mkcksum() function parameters
Dear Aaron Williams,
Quoted 8 lines
Hi Marek,
This patch is broken. It breaks detecting duplicate filenames. The problem is that you are using sizeof(name) and sizeof(ext). This does not work since this just reports the size of the pointer in gcc 4.7, which according to our compiler guy is the correct behavior. Instead of using sizeof, just use 8 and 3 respectively. Otherwise it is doing a checksum of 4 and 4 characters.
You're right. Can you post a patch for that please? This was already applied. Also, do you see any symptoms?
Best regards,
Marek Vasut