Fixed issue with trailing dots in file paths

Paths such as the following were causing issues:
/tea/hottea/.
/tea/hottea/..

Unfortunately the existing structure for path lookup didn't make it very
easy to introduce proper handling in this case without duplicating the
entire skip logic for paths. So the lfs_dir_find function had to be
restructured a bit.

One odd side-effect of this is that now lfs_dir_find includes the
initial fetch operation. This kinda breaks the fetch -> op pattern of
the dir functions, but does come with a nice code size reduction.

.travis.yml

@@ -35,7 +35,7 @@ script:
     if [ "$TRAVIS_TEST_RESULT" -eq 0 ]
     then
         CURR=$(tail -n1 sizes | awk '{print $1}')
-        PREV=$(curl https://api.github.com/repos/$TRAVIS_REPO_SLUG/status/master \
+        PREV=$(curl -u $GEKY_BOT_STATUSES https://api.github.com/repos/$TRAVIS_REPO_SLUG/status/master \
             | jq -re "select(.sha != \"$TRAVIS_COMMIT\")
                 | .statuses[] | select(.context == \"$STAGE/$NAME\").description
                 | capture(\"code size is (?<size>[0-9]+)\").size" \
@@ -165,7 +165,8 @@ jobs:
                     \"name\": \"$LFS_VERSION\"
                 }"
             RELEASE=$(
-                curl -f https://api.github.com/repos/$TRAVIS_REPO_SLUG/releases/tags/$LFS_VERSION
+                curl -f -u $GEKY_BOT_RELEASES \
+                    https://api.github.com/repos/$TRAVIS_REPO_SLUG/releases/tags/$LFS_VERSION
             )
             CHANGES=$(
                 git log --oneline $LFS_PREV_VERSION.. --grep='^Merge' --invert-grep

Makefile

@@ -33,8 +33,8 @@ size: $(OBJ)
 	$(SIZE) -t $^
 
 .SUFFIXES:
-test: test_format test_dirs test_files test_seek test_truncate test_parallel \
-	test_alloc test_paths test_orphan test_move test_corrupt
+test: test_format test_dirs test_files test_seek test_truncate \
+	test_interspersed test_alloc test_paths test_orphan test_move test_corrupt
 test_%: tests/test_%.sh
 ifdef QUIET
 	@./$< | sed -n '/^[-=]/p'

lfs.c

@@ -270,8 +270,7 @@ int lfs_deorphan(lfs_t *lfs);
 static int lfs_alloc_lookahead(void *p, lfs_block_t block) {
     lfs_t *lfs = p;
 
-    lfs_block_t off = (((lfs_soff_t)(block - lfs->free.begin)
-                % (lfs_soff_t)(lfs->cfg->block_count))
+    lfs_block_t off = ((block - lfs->free.off)
             + lfs->cfg->block_count) % lfs->cfg->block_count;
 
     if (off < lfs->free.size) {
@@ -283,27 +282,38 @@ static int lfs_alloc_lookahead(void *p, lfs_block_t block) {
 
 static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
     while (true) {
-        while (lfs->free.off != lfs->free.size) {
-            lfs_block_t off = lfs->free.off;
-            lfs->free.off += 1;
+        while (lfs->free.i != lfs->free.size) {
+            lfs_block_t off = lfs->free.i;
+            lfs->free.i += 1;
+            lfs->free.ack -= 1;
 
             if (!(lfs->free.buffer[off / 32] & (1U << (off % 32)))) {
                 // found a free block
-                *block = (lfs->free.begin + off) % lfs->cfg->block_count;
+                *block = (lfs->free.off + off) % lfs->cfg->block_count;
+
+                // eagerly find next off so an alloc ack can
+                // discredit old lookahead blocks
+                while (lfs->free.i != lfs->free.size &&
+                        (lfs->free.buffer[lfs->free.i / 32]
+                            & (1U << (lfs->free.i % 32)))) {
+                    lfs->free.i += 1;
+                    lfs->free.ack -= 1;
+                }
+
                 return 0;
             }
         }
 
         // check if we have looked at all blocks since last ack
-        if (lfs->free.off == lfs->free.ack - lfs->free.begin) {
-            LFS_WARN("No more free space %d", lfs->free.off + lfs->free.begin);
+        if (lfs->free.ack == 0) {
+            LFS_WARN("No more free space %d", lfs->free.i + lfs->free.off);
             return LFS_ERR_NOSPC;
         }
 
-        lfs->free.begin += lfs->free.size;
-        lfs->free.size = lfs_min(lfs->cfg->lookahead,
-                lfs->free.ack - lfs->free.begin);
-        lfs->free.off = 0;
+        lfs->free.off = (lfs->free.off + lfs->free.size)
+                % lfs->cfg->block_count;
+        lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->free.ack);
+        lfs->free.i = 0;
 
         // find mask of free blocks from tree
         memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8);
@@ -315,7 +325,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
 }
 
 static void lfs_alloc_ack(lfs_t *lfs) {
-    lfs->free.ack = lfs->free.off-1 + lfs->free.begin + lfs->cfg->block_count;
+    lfs->free.ack = lfs->cfg->block_count;
 }
 
 
@@ -773,26 +783,19 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir,
         lfs_entry_t *entry, const char **path) {
     const char *pathname = *path;
     size_t pathlen;
+    entry->d.type = LFS_TYPE_DIR;
+    entry->d.elen = sizeof(entry->d) - 4;
+    entry->d.alen = 0;
+    entry->d.nlen = 0;
+    entry->d.u.dir[0] = lfs->root[0];
+    entry->d.u.dir[1] = lfs->root[1];
 
     while (true) {
-    nextname:
+nextname:
         // skip slashes
         pathname += strspn(pathname, "/");
         pathlen = strcspn(pathname, "/");
 
-        // special case for root dir
-        if (pathname[0] == '\0') {
-            *entry = (lfs_entry_t){
-                .d.type = LFS_TYPE_DIR,
-                .d.elen = sizeof(entry->d) - 4,
-                .d.alen = 0,
-                .d.nlen = 0,
-                .d.u.dir[0] = lfs->root[0],
-                .d.u.dir[1] = lfs->root[1],
-            };
-            return 0;
-        }
-
         // skip '.' and root '..'
         if ((pathlen == 1 && memcmp(pathname, ".", 1) == 0) ||
             (pathlen == 2 && memcmp(pathname, "..", 2) == 0)) {
@@ -824,10 +827,25 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir,
             suffix += sufflen;
         }
 
+        // found path
+        if (pathname[0] == '\0') {
+            return 0;
+        }
+
         // update what we've found
         *path = pathname;
 
-        // find path
+        // continue on if we hit a directory
+        if (entry->d.type != LFS_TYPE_DIR) {
+            return LFS_ERR_NOTDIR;
+        }
+
+        int err = lfs_dir_fetch(lfs, dir, entry->d.u.dir);
+        if (err) {
+            return err;
+        }
+
+        // find entry matching name
         while (true) {
             int err = lfs_dir_next(lfs, dir, entry);
             if (err) {
@@ -863,21 +881,8 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir,
             entry->d.type &= ~0x80;
         }
 
+        // to next name
         pathname += pathlen;
-        pathname += strspn(pathname, "/");
-        if (pathname[0] == '\0') {
-            return 0;
-        }
-
-        // continue on if we hit a directory
-        if (entry->d.type != LFS_TYPE_DIR) {
-            return LFS_ERR_NOTDIR;
-        }
-
-        int err = lfs_dir_fetch(lfs, dir, entry->d.u.dir);
-        if (err) {
-            return err;
-        }
     }
 }
 
@@ -894,13 +899,8 @@ int lfs_mkdir(lfs_t *lfs, const char *path) {
 
     // fetch parent directory
     lfs_dir_t cwd;
-    int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t entry;
-    err = lfs_dir_find(lfs, &cwd, &entry, &path);
+    int err = lfs_dir_find(lfs, &cwd, &entry, &path);
     if (err != LFS_ERR_NOENT || strchr(path, '/') != NULL) {
         return err ? err : LFS_ERR_EXIST;
     }
@@ -944,13 +944,8 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) {
     dir->pair[0] = lfs->root[0];
     dir->pair[1] = lfs->root[1];
 
-    int err = lfs_dir_fetch(lfs, dir, dir->pair);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t entry;
-    err = lfs_dir_find(lfs, dir, &entry, &path);
+    int err = lfs_dir_find(lfs, dir, &entry, &path);
     if (err) {
         return err;
     } else if (entry.d.type != LFS_TYPE_DIR) {
@@ -1292,13 +1287,8 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file,
 
     // allocate entry for file if it doesn't exist
     lfs_dir_t cwd;
-    int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t entry;
-    err = lfs_dir_find(lfs, &cwd, &entry, &path);
+    int err = lfs_dir_find(lfs, &cwd, &entry, &path);
     if (err && (err != LFS_ERR_NOENT || strchr(path, '/') != NULL)) {
         return err;
     }
@@ -1804,13 +1794,8 @@ lfs_soff_t lfs_file_size(lfs_t *lfs, lfs_file_t *file) {
 /// General fs operations ///
 int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info) {
     lfs_dir_t cwd;
-    int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t entry;
-    err = lfs_dir_find(lfs, &cwd, &entry, &path);
+    int err = lfs_dir_find(lfs, &cwd, &entry, &path);
     if (err) {
         return err;
     }
@@ -1845,13 +1830,8 @@ int lfs_remove(lfs_t *lfs, const char *path) {
     }
 
     lfs_dir_t cwd;
-    int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t entry;
-    err = lfs_dir_find(lfs, &cwd, &entry, &path);
+    int err = lfs_dir_find(lfs, &cwd, &entry, &path);
     if (err) {
         return err;
     }
@@ -1906,24 +1886,14 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
 
     // find old entry
     lfs_dir_t oldcwd;
-    int err = lfs_dir_fetch(lfs, &oldcwd, lfs->root);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t oldentry;
-    err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath);
+    int err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath);
     if (err) {
         return err;
     }
 
     // allocate new entry
     lfs_dir_t newcwd;
-    err = lfs_dir_fetch(lfs, &newcwd, lfs->root);
-    if (err) {
-        return err;
-    }
-
     lfs_entry_t preventry;
     err = lfs_dir_find(lfs, &newcwd, &preventry, &newpath);
     if (err && (err != LFS_ERR_NOENT || strchr(newpath, '/') != NULL)) {
@@ -2094,9 +2064,9 @@ int lfs_format(lfs_t *lfs, const struct lfs_config *cfg) {
 
     // create free lookahead
     memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8);
-    lfs->free.begin = 0;
-    lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count);
     lfs->free.off = 0;
+    lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count);
+    lfs->free.i = 0;
     lfs_alloc_ack(lfs);
 
     // create superblock dir
@@ -2173,9 +2143,9 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) {
     }
 
     // setup free lookahead
-    lfs->free.begin = 0;
-    lfs->free.size = 0;
     lfs->free.off = 0;
+    lfs->free.size = 0;
+    lfs->free.i = 0;
     lfs_alloc_ack(lfs);
 
     // load superblock
@@ -2199,7 +2169,7 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) {
     }
 
     if (err || memcmp(superblock.d.magic, "littlefs", 8) != 0) {
-        LFS_ERROR("Invalid superblock at %d %d", dir.pair[0], dir.pair[1]);
+        LFS_ERROR("Invalid superblock at %d %d", 0, 1);
         return LFS_ERR_CORRUPT;
     }
 

lfs.h

@@ -259,9 +259,9 @@ typedef struct lfs_superblock {
 } lfs_superblock_t;
 
 typedef struct lfs_free {
-    lfs_block_t begin;
-    lfs_block_t size;
     lfs_block_t off;
+    lfs_block_t size;
+    lfs_block_t i;
     lfs_block_t ack;
     uint32_t *buffer;
 } lfs_free_t;
@@ -320,10 +320,6 @@ int lfs_remove(lfs_t *lfs, const char *path);
 // If the destination exists, it must match the source in type.
 // If the destination is a directory, the directory must be empty.
 //
-// Note: If power loss occurs, it is possible that the file or directory
-// will exist in both the oldpath and newpath simultaneously after the
-// next mount.
-//
 // Returns a negative error code on failure.
 int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath);
 

tests/test_alloc.sh

@@ -289,7 +289,7 @@ tests/test.py << TEST
     }
     lfs_file_close(&lfs, &file[0]) => 0;
 
-    // open whole
+    // open hole
     lfs_remove(&lfs, "bump") => 0;
 
     lfs_mkdir(&lfs, "splitdir") => 0;
@@ -301,5 +301,128 @@ tests/test.py << TEST
     lfs_unmount(&lfs) => 0;
 TEST
 
+echo "--- Outdated lookahead test ---"
+rm -rf blocks
+tests/test.py << TEST
+    lfs_format(&lfs, &cfg) => 0;
+
+    lfs_mount(&lfs, &cfg) => 0;
+
+    // fill completely with two files
+    lfs_file_open(&lfs, &file[0], "exhaustion1",
+            LFS_O_WRONLY | LFS_O_CREAT) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4)/2)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    lfs_file_open(&lfs, &file[0], "exhaustion2",
+            LFS_O_WRONLY | LFS_O_CREAT) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4+1)/2)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    // remount to force reset of lookahead
+    lfs_unmount(&lfs) => 0;
+
+    lfs_mount(&lfs, &cfg) => 0;
+
+    // rewrite one file
+    lfs_file_open(&lfs, &file[0], "exhaustion1",
+            LFS_O_WRONLY | LFS_O_TRUNC) => 0;
+    lfs_file_sync(&lfs, &file[0]) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4)/2)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    // rewrite second file, this requires lookahead does not
+    // use old population
+    lfs_file_open(&lfs, &file[0], "exhaustion2",
+            LFS_O_WRONLY | LFS_O_TRUNC) => 0;
+    lfs_file_sync(&lfs, &file[0]) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4+1)/2)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+TEST
+
+echo "--- Outdated lookahead and split dir test ---"
+rm -rf blocks
+tests/test.py << TEST
+    lfs_format(&lfs, &cfg) => 0;
+
+    lfs_mount(&lfs, &cfg) => 0;
+
+    // fill completely with two files
+    lfs_file_open(&lfs, &file[0], "exhaustion1",
+            LFS_O_WRONLY | LFS_O_CREAT) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4)/2)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    lfs_file_open(&lfs, &file[0], "exhaustion2",
+            LFS_O_WRONLY | LFS_O_CREAT) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4+1)/2)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    // remount to force reset of lookahead
+    lfs_unmount(&lfs) => 0;
+
+    lfs_mount(&lfs, &cfg) => 0;
+
+    // rewrite one file with a hole of one block
+    lfs_file_open(&lfs, &file[0], "exhaustion1",
+            LFS_O_WRONLY | LFS_O_TRUNC) => 0;
+    lfs_file_sync(&lfs, &file[0]) => 0;
+    size = strlen("blahblahblahblah");
+    memcpy(buffer, "blahblahblahblah", size);
+    for (lfs_size_t i = 0;
+            i < ((cfg.block_count-4)/2 - 1)*(cfg.block_size-8);
+            i += size) {
+        lfs_file_write(&lfs, &file[0], buffer, size) => size;
+    }
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    // try to allocate a directory, should fail!
+    lfs_mkdir(&lfs, "split") => LFS_ERR_NOSPC;
+
+    // file should not fail
+    lfs_file_open(&lfs, &file[0], "notasplit",
+            LFS_O_WRONLY | LFS_O_CREAT) => 0;
+    lfs_file_write(&lfs, &file[0], "hi", 2) => 2;
+    lfs_file_close(&lfs, &file[0]) => 0;
+
+    lfs_unmount(&lfs) => 0;
+TEST
+
 echo "--- Results ---"
 tests/stats.py

tests/test_interspersed.sh

@@ -1,13 +1,13 @@
 #!/bin/bash
 set -eu
 
-echo "=== Parallel tests ==="
+echo "=== Interspersed tests ==="
 rm -rf blocks
 tests/test.py << TEST
     lfs_format(&lfs, &cfg) => 0;
 TEST
 
-echo "--- Parallel file test ---"
+echo "--- Interspersed file test ---"
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;
     lfs_file_open(&lfs, &file[0], "a", LFS_O_WRONLY | LFS_O_CREAT) => 0;
@@ -77,7 +77,7 @@ tests/test.py << TEST
     lfs_unmount(&lfs) => 0;
 TEST
 
-echo "--- Parallel remove file test ---"
+echo "--- Interspersed remove file test ---"
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;
     lfs_file_open(&lfs, &file[0], "e", LFS_O_WRONLY | LFS_O_CREAT) => 0;

tests/test_paths.sh

@@ -90,6 +90,22 @@ tests/test.py << TEST
     lfs_unmount(&lfs) => 0;
 TEST
 
+echo "--- Trailing dot path tests ---"
+tests/test.py << TEST
+    lfs_mount(&lfs, &cfg) => 0;
+    lfs_stat(&lfs, "tea/hottea/", &info) => 0;
+    strcmp(info.name, "hottea") => 0;
+    lfs_stat(&lfs, "tea/hottea/.", &info) => 0;
+    strcmp(info.name, "hottea") => 0;
+    lfs_stat(&lfs, "tea/hottea/./.", &info) => 0;
+    strcmp(info.name, "hottea") => 0;
+    lfs_stat(&lfs, "tea/hottea/..", &info) => 0;
+    strcmp(info.name, "tea") => 0;
+    lfs_stat(&lfs, "tea/hottea/../.", &info) => 0;
+    strcmp(info.name, "tea") => 0;
+    lfs_unmount(&lfs) => 0;
+TEST
+
 echo "--- Root dot dot path tests ---"
 tests/test.py << TEST
     lfs_mount(&lfs, &cfg) => 0;