* [U-Boot] [PATCH] fs: fat: Fix mkcksum() function parameters
@ 2012-09-30 0:49 Marek Vasut
2012-10-09 16:45 ` Tom Rini
2012-10-09 17:20 ` [U-Boot] [PATCH V2] " Marek Vasut
0 siblings, 2 replies; 6+ messages in thread
From: Marek Vasut @ 2012-09-30 0:49 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
---
fs/fat/fat.c | 20 ++++++++++++--------
fs/fat/fat_write.c | 2 +-
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 41ae15e..8870292 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -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;
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 5829adf..97a18cb 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -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));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] fs: fat: Fix mkcksum() function parameters
2012-09-30 0:49 [U-Boot] [PATCH] fs: fat: Fix mkcksum() function parameters Marek Vasut
@ 2012-10-09 16:45 ` Tom Rini
2012-10-09 17:20 ` [U-Boot] [PATCH V2] " Marek Vasut
1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2012-10-09 16:45 UTC (permalink / raw)
To: u-boot
On Sun, Sep 30, 2012 at 02:49:09AM +0200, Marek Vasut wrote:
> 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.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
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.
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH V2] fs: fat: Fix mkcksum() function parameters
2012-09-30 0:49 [U-Boot] [PATCH] fs: fat: Fix mkcksum() function parameters Marek Vasut
2012-10-09 16:45 ` Tom Rini
@ 2012-10-09 17:20 ` Marek Vasut
2012-10-17 15:03 ` [U-Boot] [U-Boot, " Tom Rini
2013-01-09 1:54 ` [U-Boot] [PATCH " Aaron Williams
1 sibling, 2 replies; 6+ messages in thread
From: Marek Vasut @ 2012-10-09 17:20 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
---
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
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 41ae15e..8870292 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -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;
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 5829adf..4a1bda0 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -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));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [U-Boot, V2] fs: fat: Fix mkcksum() function parameters
2012-10-09 17:20 ` [U-Boot] [PATCH V2] " Marek Vasut
@ 2012-10-17 15:03 ` Tom Rini
2013-01-09 1:54 ` [U-Boot] [PATCH " Aaron Williams
1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2012-10-17 15:03 UTC (permalink / raw)
To: u-boot
On Tue, Oct 09, 2012 at 07:20:22AM -0000, Marek Vasut wrote:
> 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.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
Applied to u-boot/master, thanks!
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH V2] fs: fat: Fix mkcksum() function parameters
2012-10-09 17:20 ` [U-Boot] [PATCH V2] " Marek Vasut
2012-10-17 15:03 ` [U-Boot] [U-Boot, " Tom Rini
@ 2013-01-09 1:54 ` Aaron Williams
2013-01-09 9:14 ` Marek Vasut
1 sibling, 1 reply; 6+ messages in thread
From: Aaron Williams @ 2013-01-09 1:54 UTC (permalink / raw)
To: u-boot
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.
-Aaron
On 10/09/2012 10:20 AM, Marek Vasut wrote:
> 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.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
> 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
>
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 41ae15e..8870292 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -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;
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 5829adf..4a1bda0 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -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));
--
Aaron Williams
Software Engineer
Cavium, Inc.
(408) 943-7198 (510) 789-8988 (cell)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH V2] fs: fat: Fix mkcksum() function parameters
2013-01-09 1:54 ` [U-Boot] [PATCH " Aaron Williams
@ 2013-01-09 9:14 ` Marek Vasut
0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2013-01-09 9:14 UTC (permalink / raw)
To: u-boot
Dear Aaron Williams,
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-09 9:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-30 0:49 [U-Boot] [PATCH] fs: fat: Fix mkcksum() function parameters Marek Vasut
2012-10-09 16:45 ` Tom Rini
2012-10-09 17:20 ` [U-Boot] [PATCH V2] " Marek Vasut
2012-10-17 15:03 ` [U-Boot] [U-Boot, " Tom Rini
2013-01-09 1:54 ` [U-Boot] [PATCH " Aaron Williams
2013-01-09 9:14 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.