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

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

[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?

Best regards,
--
Hector Palacios

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

[Marek Vasut]

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

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

[Tom Rini]

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.

-- 
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/20131014/146f80ab/attachment.pgp>

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

[Jagan Teki]

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-implementation-td145817.html

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.

-- 
Thanks,
Jagan.
--------
Jagannadha Sutradharudu Teki,
E: jagannadh.teki at gmail.com, P: +91-9676773388
Engineer - System Software Hacker
U-boot - SPI Custodian and Zynq APSOC
Ln: http://www.linkedin.com/in/jaganteki

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

[Hector Palacios]

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-implementation-td145817.html
>
> 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

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

[Jagan Teki]

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-implementation-td145817.html
>>
>> 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?

-- 
Thanks,
Jagan.
--------
Jagannadha Sutradharudu Teki,
E: jagannadh.teki at gmail.com, P: +91-9676773388
Engineer - System Software Hacker
U-boot - SPI Custodian and Zynq APSOC
Ln: http://www.linkedin.com/in/jaganteki

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

[Albert ARIBAUD]

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-implementation-td145817.html
> >>
> >> 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,
-- 
Albert.

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

[drambo]

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-and-short-names-tp165377p168231.html
Sent from the U-Boot mailing list archive at Nabble.com.

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

[Tom Rini]

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!

-- 
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/20131122/7b66f8ce/attachment.pgp>

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

[Wolfgang Denk]

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

-- 
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
"An open mind has but one disadvantage: it collects dirt."
                                                    - a saying at RPI

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

[drambo]

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-and-short-names-tp165377p168237.html
Sent from the U-Boot mailing list archive at Nabble.com.