[U-Boot] fatls shows duplicated entries with long and short names

Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
Best regards, -- Hector Palacios

Dear Hector Palacios,
Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
CCing Tom.
Best regards, Marek Vasut

On Mon, Oct 14, 2013 at 06:00:20PM +0200, Hector Palacios wrote:
Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
Can you please provide more details about your platform and what U-Boot rev you see this on exactly? I haven't seen anything like this on Beaglebone Black recently, for example.

On Mon, Oct 14, 2013 at 9:37 PM, Tom Rini trini@ti.com wrote:
On Mon, Oct 14, 2013 at 06:00:20PM +0200, Hector Palacios wrote:
Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
Can you please provide more details about your platform and what U-Boot rev you see this on exactly? I haven't seen anything like this on Beaglebone Black recently, for example.
This kind of issue we faced(by Michal) http://u-boot.10912.n7.nabble.com/FAT-problem-with-new-mkcksum-implementatio...
Where the issue got resolved with the change from Marek "vfat: Fix mkcksum argument sizes" (sha: 6ad77d88e57f6ab815ec7e85c5ac329054318c73)
My suggestion to Hector, can you try to reproduce the same by format the card again.

Dear Jagan,
On 10/14/2013 06:57 PM, Jagan Teki wrote:
On Mon, Oct 14, 2013 at 9:37 PM, Tom Rini trini@ti.com wrote:
On Mon, Oct 14, 2013 at 06:00:20PM +0200, Hector Palacios wrote:
Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
Can you please provide more details about your platform and what U-Boot rev you see this on exactly? I haven't seen anything like this on Beaglebone Black recently, for example.
This kind of issue we faced(by Michal) http://u-boot.10912.n7.nabble.com/FAT-problem-with-new-mkcksum-implementatio...
Where the issue got resolved with the change from Marek "vfat: Fix mkcksum argument sizes" (sha: 6ad77d88e57f6ab815ec7e85c5ac329054318c73)
I was testing on v2013.01. This patch fixes it. Thank you.
Best regards, -- Hector Palacios

On Tue, Oct 15, 2013 at 12:46 PM, Hector Palacios hector.palacios@digi.com wrote:
Dear Jagan,
On 10/14/2013 06:57 PM, Jagan Teki wrote:
On Mon, Oct 14, 2013 at 9:37 PM, Tom Rini trini@ti.com wrote:
On Mon, Oct 14, 2013 at 06:00:20PM +0200, Hector Palacios wrote:
Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
Can you please provide more details about your platform and what U-Boot rev you see this on exactly? I haven't seen anything like this on Beaglebone Black recently, for example.
This kind of issue we faced(by Michal)
http://u-boot.10912.n7.nabble.com/FAT-problem-with-new-mkcksum-implementatio...
Where the issue got resolved with the change from Marek "vfat: Fix mkcksum argument sizes" (sha: 6ad77d88e57f6ab815ec7e85c5ac329054318c73)
I was testing on v2013.01. This patch fixes it. Thank you.
Does this means master still have an issue?

Hi Jagan,
On Tue, 15 Oct 2013 12:51:19 +0530, Jagan Teki jagannadh.teki@gmail.com wrote:
On Tue, Oct 15, 2013 at 12:46 PM, Hector Palacios hector.palacios@digi.com wrote:
Dear Jagan,
On 10/14/2013 06:57 PM, Jagan Teki wrote:
On Mon, Oct 14, 2013 at 9:37 PM, Tom Rini trini@ti.com wrote:
On Mon, Oct 14, 2013 at 06:00:20PM +0200, Hector Palacios wrote:
Dear Marek,
I noticed that 'fatls' displays duplicated filenames (short and long) for every file in the media:
# fatls mmc 0 2083460 uimage-myplatform 2083460 uimage~1 1520 rootfs-dummy.jffs2 1520 rootfs~1.jff 3294952 uimage 3294952 uimage
The guilty commit is ff04f6d1224d8952b566b8671222151495883073 by you, who moved the chksum calculation out of an if() and now the code never enters this:
#ifdef CONFIG_SUPPORT_VFAT else if (dols == LS_ROOT && csum == prevcksum) { prevcksum = 0xffff; dentptr++; continue; } #endif
Could you please check?
Can you please provide more details about your platform and what U-Boot rev you see this on exactly? I haven't seen anything like this on Beaglebone Black recently, for example.
This kind of issue we faced(by Michal)
http://u-boot.10912.n7.nabble.com/FAT-problem-with-new-mkcksum-implementatio...
Where the issue got resolved with the change from Marek "vfat: Fix mkcksum argument sizes" (sha: 6ad77d88e57f6ab815ec7e85c5ac329054318c73)
I was testing on v2013.01. This patch fixes it. Thank you.
Does this means master still have an issue?
Not since 6ad77d88 went in. Commit itself is dated 11th january , and was applied to u-boot/master on jan. 31st.
Amicalement,

Hello,
If it helps, I would respectfully like to add that I reverted this change and it fixes my problem nicely. I'm on an ARM platform (not yet upstreamed to u-boot), with a microSD card inserted with valid files on it that I can read on other systems. I have an Oct 16, 2013 version of U-Boot 2013.10
Before I was seeing: capri> fatls mmc 1 dcim/106gopro ./ ../ 486383878 gopr0150.mp4 486383878 gopr0150.mp4 185120785 gopr0151.mp4 185120785 gopr0151.mp4 75273257 gopr0152.mp4 75273257 gopr0152.mp4 660782172 gopr0153.mp4 660782172 gopr0153.mp4 735056468 gopr0154.mp4 735056468 gopr0154.mp4 7571812 gopr0155.jpg 7571812 gopr0155.jpg 7670056 gopr0156.jpg 7670056 gopr0156.jpg 7801014 gopr0157.jpg 7801014 gopr0157.jpg 172345390 gopr0158.mp4 172345390 gopr0158.mp4 1501760194 gopr0159.mp4 1501760194 gopr0159.mp4 56610361 gopr0160.mp4 56610361 gopr0160.mp4 1069538258 gopr0161.mp4 1069538258 gopr0161.mp4 7301275 gopr0162.jpg 7301275 gopr0162.jpg 1890783532 gopr0163.mp4 1890783532 gopr0163.mp4
28 file(s), 2 dir(s)
Now by putting the checksum calculation inside the if statement I am seeing: capri> fatls mmc 1 dcim/106gopro ./ ../ 486383878 gopr0150.mp4 185120785 gopr0151.mp4 75273257 gopr0152.mp4 660782172 gopr0153.mp4 735056468 gopr0154.mp4 7571812 gopr0155.jpg 7670056 gopr0156.jpg 7801014 gopr0157.jpg 172345390 gopr0158.mp4 1501760194 gopr0159.mp4 56610361 gopr0160.mp4 1069538258 gopr0161.mp4 7301275 gopr0162.jpg 1890783532 gopr0163.mp4
14 file(s), 2 dir(s)
I sincerely hope this helps the discussion. Best Regards, Darwin Rambo Broadcom Corporation
-- View this message in context: http://u-boot.10912.n7.nabble.com/fatls-shows-duplicated-entries-with-long-a... Sent from the U-Boot mailing list archive at Nabble.com.

On Fri, Nov 22, 2013 at 11:31:29AM -0800, drambo wrote:
Hello,
If it helps, I would respectfully like to add that I reverted this change and it fixes my problem nicely. I'm on an ARM platform (not yet upstreamed to u-boot), with a microSD card inserted with valid files on it that I can read on other systems. I have an Oct 16, 2013 version of U-Boot 2013.10
Before I was seeing: capri> fatls mmc 1 dcim/106gopro ./ ../ 486383878 gopr0150.mp4 486383878 gopr0150.mp4 185120785 gopr0151.mp4 185120785 gopr0151.mp4 75273257 gopr0152.mp4 75273257 gopr0152.mp4 660782172 gopr0153.mp4 660782172 gopr0153.mp4 735056468 gopr0154.mp4 735056468 gopr0154.mp4 7571812 gopr0155.jpg 7571812 gopr0155.jpg 7670056 gopr0156.jpg 7670056 gopr0156.jpg 7801014 gopr0157.jpg 7801014 gopr0157.jpg 172345390 gopr0158.mp4 172345390 gopr0158.mp4 1501760194 gopr0159.mp4 1501760194 gopr0159.mp4 56610361 gopr0160.mp4 56610361 gopr0160.mp4 1069538258 gopr0161.mp4 1069538258 gopr0161.mp4 7301275 gopr0162.jpg 7301275 gopr0162.jpg 1890783532 gopr0163.mp4 1890783532 gopr0163.mp4
28 file(s), 2 dir(s)
Now by putting the checksum calculation inside the if statement I am seeing: capri> fatls mmc 1 dcim/106gopro ./ ../ 486383878 gopr0150.mp4 185120785 gopr0151.mp4 75273257 gopr0152.mp4 660782172 gopr0153.mp4 735056468 gopr0154.mp4 7571812 gopr0155.jpg 7670056 gopr0156.jpg 7801014 gopr0157.jpg 172345390 gopr0158.mp4 1501760194 gopr0159.mp4 56610361 gopr0160.mp4 1069538258 gopr0161.mp4 7301275 gopr0162.jpg 1890783532 gopr0163.mp4
14 file(s), 2 dir(s)
I sincerely hope this helps the discussion. Best Regards,
So, the last report said that they had a tree missing 6ad77d88e57f6ab815ec7e85c5ac329054318c73, but yours can't be. Can you add some debug prints and see what's going on? Having u8 foo = bar(); if (a && foo == baz) { ... } not work, but: if (a && bar() == baz) { ... } work is quite puzzling. Thanks!

Dear Tom,
In message 20131122205046.GU420@bill-the-cat you wrote:
So, the last report said that they had a tree missing 6ad77d88e57f6ab815ec7e85c5ac329054318c73, but yours can't be. Can you add some debug prints and see what's going on? Having u8 foo = bar(); if (a && foo == baz) { ... } not work, but: if (a && bar() == baz) { ... } work is quite puzzling. Thanks!
The actual code is this:
if (dols && mkcksum(dentptr->name) == prevcksum) {
versus
__u8 csum = mkcksum(dentptr->name, dentptr->ext); if (dols && csum == prevcksum) {
Note that csum is __u8, but prevcksum is __u16. Eventually there is a type issue. It looks a bit fishy to me that prevcksum 16 bits, while all operations are done on 8 bit data, and comparison is against 8 bit as well.
Best regards,
Wolfgang Denk

I reverted my local change and the directory listing shows no duplicates anymore which is surprising. An inspection of the generated assembly shows more or less the same code with the exception of the early branch taken if 'dols' is 0. The 16/8 comparison is still awkward, but at this point I'm not confident that is the real problem here. The changing result with the same original code seems to point to perhaps a different root cause.
Here's the original assembly: if (vfat_enabled) { __u8 csum = mkcksum(dentptr->name, dentptr->ext) ; ae012560: e1a00004 mov r0, r4 ae012564: e2841008 add r1, r4, #8 ae012568: ebfffc81 bl ae011774 <mkcksum> if (dols && csum == prevcksum) { ae01256c: e59dc010 ldr ip, [sp, #16] ae012570: e35c0000 cmp ip, #0 ae012574: 0a000005 beq ae012590 <do_fat_read_at+0x718> ae012578: e59dc024 ldr ip, [sp, #36] ; 0x24 ae01257c: e150000c cmp r0, ip prevcksum = 0xffff; ae012580: 030fcfff movweq ip, #65535 ; 0xffff dentptr++; ae012584: 02844020 addeq r4, r4, #32 return NULL; } if (vfat_enabled) { __u8 csum = mkcksum(dentptr->name, dentptr->ext) ; if (dols && csum == prevcksum) { prevcksum = 0xffff; ae012588: 058dc024 streq ip, [sp, #36] ; 0x24 ae01258c: 0a000035 beq ae012668 <do_fat_read_at+0x7f0> dentptr++; continue; } }
Here's the version without the local crc variable: if (vfat_enabled) { if (dols && mkcksum(dentptr->name, dentptr->ext) == prevcksum) { ae012560: e59dc010 ldr ip, [sp, #16] ae012564: e35c0000 cmp ip, #0 ae012568: 0a000008 beq ae012590 <do_fat_read_at+0x718> ae01256c: e1a00004 mov r0, r4 ae012570: e2841008 add r1, r4, #8 ae012574: ebfffc7e bl ae011774 <mkcksum> ae012578: e59dc024 ldr ip, [sp, #36] ; 0x24 ae01257c: e150000c cmp r0, ip prevcksum = 0xffff; ae012580: 030fcfff movweq ip, #65535 ; 0xffff dentptr++; ae012584: 02844020 addeq r4, r4, #32 debug("Dentname == NULL - %d\n", i); return NULL; } if (vfat_enabled) { if (dols && mkcksum(dentptr->name, dentptr->ext) == prevcksum) { prevcksum = 0xffff; ae012588: 058dc024 streq ip, [sp, #36] ; 0x24 ae01258c: 0a000035 beq ae012668 <do_fat_read_at+0x7f0> dentptr++; continue; } }
-- View this message in context: http://u-boot.10912.n7.nabble.com/fatls-shows-duplicated-entries-with-long-a... Sent from the U-Boot mailing list archive at Nabble.com.
participants (7)
-
Albert ARIBAUD
-
drambo
-
Hector Palacios
-
Jagan Teki
-
Marek Vasut
-
Tom Rini
-
Wolfgang Denk