fs-test: Add FAT16 support

If we end up back in the root directory via a '..' directory entry, set itr->is_root accordingly. Failing to do that gives spews like "Invalid FAT entry" and being unable to access directory entries located past the first cluster of the root directory on FAT12/FAT16.

Fixes 8eafae209c35 ("fat/fs: convert to directory iterators")

Signed-off-by Tuomas Tynkkynen <[email protected]>

The previous commit fixed a problem in FAT code where going back to the root directory using '..' wouldn't work correctly on FAT12 or FAT16. Add a test to exercise this case (which was once fixed in commit 18a10d46f26 "fat: handle paths that include ../" but reintroduced due to the directory iterator refactoring).

This test only very barely catches the problem - without the fix the size command still gives valid output but the additional spurious "Invalid FAT entry" error message makes it not get caught in the 'egrep -A3 ' output. I tried to make a proper test that grows the root directory to two clusters lots of with dummy files but that causes the write tests to crash the sandbox totally...

Quoted 6 lines · Tuomas Tynkkynen

The previous commit fixed a problem in FAT code where going back to the root directory using '..' wouldn't work correctly on FAT12 or FAT16. Add a test to exercise this case (which was once fixed in commit 18a10d46f26 "fat: handle paths that include ../" but reintroduced due to the directory iterator refactoring).

Quoted 6 lines

This test only very barely catches the problem - without the fix the size command still gives valid output but the additional spurious "Invalid FAT entry" error message makes it not get caught in the 'egrep -A3 ' output. I tried to make a proper test that grows the root directory to two clusters lots of with dummy files but that causes the write tests to crash the sandbox totally...

Can you post, unrelated, the code that totally crashed sandbox? Maybe that's a problem we need to fix too :)

Otherwise fine, but we need to update the expected results.

--
Tom

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Quoted 4 lines · Tom Rini

On Mon, Sep 25, 2017 at 12:07:36AM +0300, Tuomas Tynkkynen wrote:

Can you post, unrelated, the code that totally crashed sandbox? Maybe that's a problem we need to fix too :)

jfwiw, I've started looking at the fat_write code, so even a failing/crashing test is useful.

867 /*

 868                  * In FAT16/12, the root dir is locate before data area, shows
 869                  * in following:
 870                  * -------------------------------------------------------------
 871                  * | Boot | FAT1 & 2 | Root dir | Data (start from cluster #2) |
 872                  * -------------------------------------------------------------
 873                  *
 874                  * As a result if curclust is in Root dir, it is a negative
 875                  * number or 0, 1.
 876                  *
 877                  */

878 if (mydata->fatsize != 32 && (int)curclust <= 1) {
879 /* Current clust is in root dir, set to next clust */
880 curclust++;

Currently we can only test FAT32 which is the default FAT version that mkfs.vfat creates by default. Instead make it explicitly create either a FAT16 or a FAT32 volume. This allows us to exercise more code, for instance the root directory handling is done differently in FAT32 than the older FATs.

Adding FAT12 support is a much bigger job since the test creates a 2.5GB file and the FAT12 maximum partition size is way smaller than that.

Currently we can only test FAT32 which is the default FAT version that mkfs.vfat creates by default. Instead make it explicitly create either a FAT16 or a FAT32 volume. This allows us to exercise more code, for instance the root directory handling is done differently in FAT32 than the older FATs.

Adding FAT12 support is a much bigger job since the test creates a 2.5GB file and the FAT12 maximum partition size is way smaller than that.

128 should do the trick.. there might end up being a lower limit on

Signed-off-by Tuomas Tynkkynen <[email protected]>

---
 test/fs/fs-test.sh | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

test/fs/fs-test.sh +27 -15

index b19486419e..2ddac50f90 100755
@@ -41,7 +41,7 @@ SMALL_FILE="1MB.file"
 BIG_FILE="2.5GB.file"
 
 # $MD5_FILE will have the expected md5s when we do the test
-# They shall have a suffix which represents their file system (ext4/fat)
+# They shall have a suffix which represents their file system (ext4/fat16/...)
 MD5_FILE="${OUT_DIR}/md5s.list"
 
 # $OUT shall be the prefix of the test output. Their suffix will be .out

@@ -104,15 +104,25 @@ function prepare_env() {
 }
 
 # 1st parameter is the name of the image file to be created
-# 2nd parameter is the filesystem - fat ext4 etc
+# 2nd parameter is the filesystem - fat16 ext4 etc
 # -F cant be used with fat as it means something else.
 function create_image() {
        # Create image if not already present - saves time, while debugging
-       if [ "$2" = "ext4" ]; then
+       case "$2" in
+               fat16)
+               MKFS_OPTION="-F 16"
+               FS_TYPE="fat"
+               ;;
+               fat32)
+               MKFS_OPTION="-F 32"
+               FS_TYPE="fat"
+               ;;
+               ext4)
                MKFS_OPTION="-F"
-       else
-               MKFS_OPTION=""
-       fi
+               FS_TYPE="ext4"
+               ;;
+       esac
+
        if [ ! -f "$1" ]; then
                fallocate -l 3G "$1" &> /dev/null
                if [ $? -ne 0 ]; then

@@ -123,8 +133,8 @@ function create_image() {
                                exit $?
                        fi
                fi
-               mkfs -t "$2" $MKFS_OPTION "$1" &> /dev/null
-               if [ $? -ne 0 -a "$2" = "fat" ]; then
+               mkfs -t "$FS_TYPE" $MKFS_OPTION "$1" &> /dev/null
+               if [ $? -ne 0 -a "$FS_TYPE" = "fat" ]; then
                        # If we fail and we did fat, try vfat.
                        mkfs -t vfat $MKFS_OPTION "$1" &> /dev/null
                fi

@@ -136,7 +146,7 @@ function create_image() {
 }
 
 # 1st parameter is image file
-# 2nd parameter is file system type - fat/ext4
+# 2nd parameter is file system type - fat16/ext4/...
 # 3rd parameter is name of small file
 # 4th parameter is name of big file
 # 5th parameter is fs/nonfs/sb - to dictate generic fs commands or

@@ -149,7 +159,7 @@ function test_image() {
        length="0x00100000"
 
        case "$2" in
-               fat)
+               fat*)
                FPATH=""
                PREFIX="fat"
                WRITE="write"

@@ -550,7 +560,7 @@ TOTAL_PASS=0
 # In each loop, for a given file system image, we test both the
 # fs command, like load/size/write, the file system specific command
 # like: ext4load/ext4size/ext4write and the sb load/ls/save commands.
-for fs in ext4 fat; do
+for fs in ext4 fat16 fat32; do
 
        echo "Creating $fs image if not already present."
        IMAGE=${IMG}.${fs}.img

@@ -563,11 +573,14 @@ for fs in ext4 fat; do
 
        # Lets mount the image and test sb hostfs commands
        mkdir -p "$MOUNT_DIR"
-       if [ "$fs" = "fat" ]; then
+       case "$fs" in
+               fat*)
                uid="uid=`id -u`"
-       else
+               ;;
+               *)
                uid=""
-       fi
+               ;;
+       esac
        sudo mount -o loop,rw,$uid "$IMAGE" "$MOUNT_DIR"
        sudo chmod 777 "$MOUNT_DIR"
 
--

2.13.0