[U-Boot] fatls returncode

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 }

Dear Mirza,
In message CAJ=nTsvH4XPk_b0SWqHX3gbVsqCbCxro=O20fQ1rr2dcgXQQMg@mail.gmail.com you wrote:
I am running U-boot 2015.04 and experiencing some issues with "fatls" command.
...
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

Dear Wolfgang.
2017-03-23 16:13 GMT+01:00 Wolfgang Denk wd@denx.de:
Dear Mirza,
In message CAJ=nTsvH4XPk_b0SWqHX3gbVsqCbCxro=O20fQ1rr2dcgXQQMg@mail.gmail.com you wrote:
I am running U-boot 2015.04 and experiencing some issues with "fatls" command.
...
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
I did an update (cherry-picked FAT related commits from upstream), but I still get the same result.
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. In my case I never reach any of these three, since my exit path is:
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 }
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.

Dear Mirza,
In message CAJ=nTstVijhpwMh9HfCbnzBzRz4GA9aujq=4oSWAkYCUs1+pOA@mail.gmail.com you wrote:
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 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.
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. In my case I never reach any of these three, since my exit path is:
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 }
OK... Understood.
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 sshtylyov@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

2017-03-27 10:51 GMT+02:00 Wolfgang Denk wd@denx.de:
Dear Mirza,
In message CAJ=nTstVijhpwMh9HfCbnzBzRz4GA9aujq=4oSWAkYCUs1+pOA@mail.gmail.com you wrote:
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 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.
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. In my case I never reach any of these three, since my exit path is:
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 }
OK... Understood.
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 sshtylyov@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?
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.
participants (2)
-
Mirza Krak
-
Wolfgang Denk