From 52df70c669244a92ee011282c1e438a1699ac4ac Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Wed, 17 Jan 2024 14:38:04 -0600
Subject: [PATCH] Fixed name ordering when names only differ in length

Wild this hasn't been caught until now.

Because the exact ordering of the comparison in lfs_bd_cmp is a bit
ambiguous, lfs_dir_find_match returned the wrong result when filenames
were equal, and only differed in length.

For example:

  - cmp("a", "aa") should be LFS_CMP_LT
  - cmp("aaa", "aa") should be LFS_CMP_GT

We're quite lucky that none of the littlefs internals currently depend
on the sorted order, otherwise we'd probably be stuck with this weird
ordering for backwards compatibility reasons...

Fixed, and added some test cases over directory ordering to prevent
regression in the future.

Found by andriyndev
---
 lfs.c                |  4 +-
 tests/test_dirs.toml | 99 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 2 deletions(-)

diff --git a/lfs.c b/lfs.c
index a152687f..49897cc4 100644
--- a/lfs.c
+++ b/lfs.c
@@ -1430,8 +1430,8 @@ static int lfs_dir_find_match(void *data,
     }
 
     // only equal if our size is still the same
-    if (name->size != lfs_tag_size(tag)) {
-        return (name->size < lfs_tag_size(tag)) ? LFS_CMP_LT : LFS_CMP_GT;
+    if (lfs_tag_size(tag) != name->size) {
+        return (lfs_tag_size(tag) < name->size) ? LFS_CMP_LT : LFS_CMP_GT;
     }
 
     // found a match!
diff --git a/tests/test_dirs.toml b/tests/test_dirs.toml
index 4262a1aa..06363d5b 100644
--- a/tests/test_dirs.toml
+++ b/tests/test_dirs.toml
@@ -717,6 +717,105 @@ code = '''
     lfs_unmount(&lfs) => 0;
 '''
 
+# littlefs should keep directories in lexicographic order
+[cases.test_dirs_ordering]
+# ORDER=0 => inorder
+# ORDER=1 => reversed
+# ORDER=2 => random
+defines.ORDER = [0, 1, 2]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    if (ORDER == 0) {
+        lfs_mkdir(&lfs, "a") => 0;
+        lfs_mkdir(&lfs, "b") => 0;
+        lfs_mkdir(&lfs, "c") => 0;
+    } else if (ORDER == 1) {
+        lfs_mkdir(&lfs, "c") => 0;
+        lfs_mkdir(&lfs, "b") => 0;
+        lfs_mkdir(&lfs, "a") => 0;
+    } else if (ORDER == 2) {
+        // "random"
+        lfs_mkdir(&lfs, "a") => 0;
+        lfs_mkdir(&lfs, "c") => 0;
+        lfs_mkdir(&lfs, "b") => 0;
+    }
+
+    // check the order
+    lfs_dir_t dir;
+    lfs_dir_open(&lfs, &dir, "/") => 0;
+    struct lfs_info info;
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, ".") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "..") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "a") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "b") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "c") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 0;
+    lfs_dir_close(&lfs, &dir) => 0;
+
+    lfs_unmount(&lfs) => 0;
+'''
+
+[cases.test_dirs_ordering_length]
+# ORDER=0 => inorder
+# ORDER=1 => reversed
+# ORDER=2 => random
+defines.ORDER = [0, 1, 2]
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    if (ORDER == 0) {
+        lfs_mkdir(&lfs, "a") => 0;
+        lfs_mkdir(&lfs, "aa") => 0;
+        lfs_mkdir(&lfs, "aaa") => 0;
+    } else if (ORDER == 1) {
+        lfs_mkdir(&lfs, "aaa") => 0;
+        lfs_mkdir(&lfs, "aa") => 0;
+        lfs_mkdir(&lfs, "a") => 0;
+    } else if (ORDER == 2) {
+        // "random"
+        lfs_mkdir(&lfs, "a") => 0;
+        lfs_mkdir(&lfs, "aaa") => 0;
+        lfs_mkdir(&lfs, "aa") => 0;
+    }
+
+    // check the order
+    lfs_dir_t dir;
+    lfs_dir_open(&lfs, &dir, "/") => 0;
+    struct lfs_info info;
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, ".") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "..") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "a") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "aa") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 1;
+    assert(info.type == LFS_TYPE_DIR);
+    assert(strcmp(info.name, "aaa") == 0);
+    lfs_dir_read(&lfs, &dir, &info) => 0;
+    lfs_dir_close(&lfs, &dir) => 0;
+
+    lfs_unmount(&lfs) => 0;
+'''
+
 [cases.test_dirs_other_errors]
 code = '''
     lfs_t lfs;