Discussion:
[U-Boot] fatls returncode
Mirza Krak
2017-03-23 08:37:37 UTC
Permalink
Hi.

I am running U-boot 2015.04 and experiencing some issues with "fatls" command.

We use the "fatls" command to detect if there is an USB storage device
plugged in the port during boot (might be better ways of doing this).
So on boot we have:

fatls usb 0:1 && <do something>

Normally "fatls" returns "0" when it does a successful list. But we
have found a case where it does not even though everything seems to
work fine. Below is a log with debug enabled (obfuscated filenames
since I got this flash drive from a customer).

MX-4 C61 # fatls usb 0:1
VFAT Support enabled
FAT32, fat_sect: 1146, fatlength: 7619
Rootdir begins at cluster: 2, sector: 16384, offset: 800000
Data begins at: 16368
Sector size: 512, cluster size: 8
FAT read(sect=16384, cnt:8), clust_size=8, DIRENTSPERBLOCK=16
system volume information/
bin/
boot/
etc/
i4m/
lib/
sbin/
share/
1530 file1.sh
15542 file2.sh
1745 file3.sh
4609 file4.sh
END LOOP: buffer_blk_cnt=0 clust_size=8
1547 file5.sh
22361 file6.sh
385078 file7.sh
33279 file8.sh
1152054 file9.sh
158132 file10.sh
1473 file11.sh
9739 file12.sh
318780 file13.sh
END LOOP: buffer_blk_cnt=1 clust_size=8
34027520 file14.sh
1246 file15.sh
1246 file16.sh
END LOOP: buffer_blk_cnt=2 clust_size=8
END LOOP: buffer_blk_cnt=3 clust_size=8
END LOOP: buffer_blk_cnt=4 clust_size=8
END LOOP: buffer_blk_cnt=5 clust_size=8
END LOOP: buffer_blk_cnt=6 clust_size=8
END LOOP: buffer_blk_cnt=7 clust_size=8
FAT32: entry: 0x0002 = 2, offset: 0x0002 = 2
FAT32: ret: 0fffffff, offset: 0002

16 file(s), 8 dir(s)

MX-4 C61 # echo $?
1

Even though the file list seems to be OK, the return code indicates an error.

I am wondering if we need to set "ret = 0" in below code-path (which
the exit path in above output)

fs/fat/fat.c:

1139 /* If end of rootdir reached */
1140 if (rootdir_end) {
1141 if (dols == LS_ROOT) {
1142 printf("\n%d file(s), %d dir(s)\n\n",
1143 files, dirs);
1144 *size = 0;
1145 }
1146 goto exit;
1147 }
--
Med Vänliga Hälsningar / Best Regards

*******************************************************************
Mirza Krak
Host Mobility AB
***@hostmobility.com
Anders Personsgatan 12, 416 64 Göteborg
Sweden
http://www.hostmobility.com
Direct: +46 31 31 32 704
Phone: +46 31 31 32 700
Fax: +46 31 80 67 51
Mobile: +46 730 28 06 22
*******************************************************************
Wolfgang Denk
2017-03-23 15:13:35 UTC
Permalink
Dear Mirza,
Post by Mirza Krak
I am running U-boot 2015.04 and experiencing some issues with "fatls" command.
...
Post by Mirza Krak
Normally "fatls" returns "0" when it does a successful list. But we
have found a case where it does not even though everything seems to
work fine. Below is a log with debug enabled (obfuscated filenames
since I got this flash drive from a customer).
Please update to a recent versionof the code. The return code
handling has probably been fixed by this commit:

0a04ed8 2015-09-11 17:15:21 -0400 FIX: fat: Provide correct return code from disk_{read|write} to upper layers


Best regards,

Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: ***@denx.de
Prof: So the American government went to IBM to come up with a
data encryption standard and they came up with ...
Student: EBCDIC!
Wolfgang Denk
2017-03-27 08:51:52 UTC
Permalink
Dear Mirza,
Post by Wolfgang Denk
Post by Wolfgang Denk
Please update to a recent versionof the code. The return code
0a04ed8 2015-09-11 17:15:21 -0400 FIX: fat: Provide correct return co=
de from disk_{read|write} to upper layers
I did an update (cherry-picked FAT related commits from upstream), but
I still get the same result.
I see.
Post by Wolfgang Denk
Analyzing the code in fs/fat/fat.c:do_fat_read_at (which is called by
the fatls command) one can see that "ret" is set in three locations.
1139 /* If end of rootdir reached */
1140 if (rootdir_end) {
1141 if (dols == LS_ROOT) {
1142 printf("\n%d file(s), %d dir(s)\n\n",
1143 files, dirs);
1144 *size = 0;
1145 }
1146 goto exit;
1147 }
OK... Understood.
Post by Wolfgang Denk
So either this exit path is actually an error and it is correct as-is,
or this path should set "ret = 0". My knowledge of FAT is limited so I
can not really tell which it should be but there is no indications in
the code/comments that this exit path is an error.
I think the exit is OK, as we have reached the end of the root
directory, but it should set "ret = 0;" before the goto.

But then, I am not an expert on this code either, so I added Sergei
Shtylyov on cc: who added this line with

commit ac4977719e157bcb3c45c70d9dd781164727530d
Author: Sergei Shtylyov <***@ru.mvista.com>
Date: Mon Aug 8 09:38:33 2011 +0000

fat: fix crash with big sector size


Sergei, do you agree that ret = 0 should be set before the "goto
exit" here?

Best regards,

Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: ***@denx.de
There is a biblical analogy I'd like to draw here. Casts are to C++
Programmers what the apple was to Eve. - Scott Douglas Meyers
Mirza Krak
2017-04-03 06:14:29 UTC
Permalink
Post by Wolfgang Denk
Dear Mirza,
Post by Wolfgang Denk
Post by Wolfgang Denk
Please update to a recent versionof the code. The return code
0a04ed8 2015-09-11 17:15:21 -0400 FIX: fat: Provide correct return co=
de from disk_{read|write} to upper layers
I did an update (cherry-picked FAT related commits from upstream), but
I still get the same result.
I see.
Post by Wolfgang Denk
Analyzing the code in fs/fat/fat.c:do_fat_read_at (which is called by
the fatls command) one can see that "ret" is set in three locations.
1139 /* If end of rootdir reached */
1140 if (rootdir_end) {
1141 if (dols == LS_ROOT) {
1142 printf("\n%d file(s), %d dir(s)\n\n",
1143 files, dirs);
1144 *size = 0;
1145 }
1146 goto exit;
1147 }
OK... Understood.
Post by Wolfgang Denk
So either this exit path is actually an error and it is correct as-is,
or this path should set "ret = 0". My knowledge of FAT is limited so I
can not really tell which it should be but there is no indications in
the code/comments that this exit path is an error.
I think the exit is OK, as we have reached the end of the root
directory, but it should set "ret = 0;" before the goto.
But then, I am not an expert on this code either, so I added Sergei
Shtylyov on cc: who added this line with
commit ac4977719e157bcb3c45c70d9dd781164727530d
Date: Mon Aug 8 09:38:33 2011 +0000
fat: fix crash with big sector size
Sergei, do you agree that ret = 0 should be set before the "goto
exit" here?
I also found the commit that removed the "ret = 0" since it did indeed
exist earlier.

1ad0b98a067a ("fat: Prepare API change for files greater than 2GB")

Added author on CC.

That commit also removed another occurrence of "ret = 0" which I
question if that was correct as well.
--
Med Vänliga Hälsningar / Best Regards

*******************************************************************
Mirza Krak
Host Mobility AB
***@hostmobility.com
Anders Personsgatan 12, 416 64 Göteborg
Sweden
http://www.hostmobility.com
Direct: +46 31 31 32 704
Phone: +46 31 31 32 700
Fax: +46 31 80 67 51
Mobile: +46 730 28 06 22
*******************************************************************
Loading...