Support removal of non-empty folders via lfs_atomic_*(...)

Introduce the function lfs_atomic_rename(...) and lfs_atomic_remove(...)
that permits the removal of a folder with multiple files within it.  This
fails when the directory contains other directories.

Author: tim-nordell-nimbelink

lfs.c

@@ -3884,6 +3884,9 @@ struct lfs_remove_state
     lfs_mdir_t cwd;
     lfs_stag_t tag;
 
+    // Global state accumulations from a split folder
+    lfs_gstate_t gstate;
+
     // Directory that needs to be removed from linked list pointer
     struct lfs_mlist dir;
 
@@ -3892,10 +3895,61 @@ struct lfs_remove_state
     // the directory.
     lfs_block_t dir_head[2];
 };
+typedef int (*lfs_dir_prep_helper_t)(lfs_t *, struct lfs_remove_state *);
 #endif
 
 #ifndef LFS_READONLY
-static int lfs_dir_prep_removal(lfs_t *lfs, struct lfs_remove_state *p) {
+static int lfs_dir_prep_remove_nonempty_folders(lfs_t *lfs, struct lfs_remove_state *p) {
+    uint16_t id = 0;
+
+    // Walk tags stored in this directory and check for any directory
+    // tags.  Removal of directories with a directory in them can lead
+    // to additional orphans in the filesystem, so we return
+    // LFS_ERR_NOTEMPTY in this case.  Otherwise, leave the loaded
+    // directory for the tail end of the directory split to leave a proper
+    // view of the filesystem after removal.
+    while(true) {
+        if (p->dir.m.count == id) {
+            if (!p->dir.m.split) {
+                // We have iterated through the folder to the last
+                // tag.
+                break;
+            }
+
+            // Before we fetch the next block, update our view of gstate
+            lfs_dir_getgstate(lfs, &p->dir.m, &p->gstate);
+
+            int err = lfs_dir_fetch(lfs, &p->dir.m, p->dir.m.tail);
+            if (err) {
+                return err;
+            }
+
+            id = 0;
+        }
+
+        char name[lfs->name_max + 1];
+        lfs_stag_t tag = lfs_dir_get(lfs, &p->dir.m, LFS_MKTAG(0x780, 0x3ff, 0),
+                LFS_MKTAG(LFS_TYPE_NAME, id, lfs->name_max + 1), name);
+        if (tag < 0) {
+            return tag;
+        }
+
+        if (lfs_tag_type3(tag) == LFS_TYPE_DIR) {
+            // We're not allowed to find leafs
+            return LFS_ERR_NOTEMPTY;
+        }
+
+        id += 1;
+    }
+
+    return 0;
+}
+#endif
+
+#ifndef LFS_READONLY
+static int lfs_dir_prep_removal(lfs_t *lfs, struct lfs_remove_state *p,
+        lfs_dir_prep_helper_t helper) {
+
     lfs_stag_t res = lfs_dir_get(lfs, &p->cwd, LFS_MKTAG(0x700, 0x3ff, 0),
             LFS_MKTAG(LFS_TYPE_STRUCT, lfs_tag_id(p->tag), 8), p->dir_head);
     if (res < 0) {
@@ -3908,8 +3962,19 @@ static int lfs_dir_prep_removal(lfs_t *lfs, struct lfs_remove_state *p) {
         return err;
     }
 
+    memset(&p->gstate, 0, sizeof(p->gstate));
+
     if (p->dir.m.count > 0 || p->dir.m.split) {
-        return LFS_ERR_NOTEMPTY;
+        // Normal POSIX behavior wouldn't allow a non-empty
+        // folder to be removed/renamed into in this manner
+        if (NULL == helper) {
+            return LFS_ERR_NOTEMPTY;
+        }
+
+        err = helper(lfs, p);
+        if (err) {
+            return err;
+        }
     }
 
     // mark fs as orphaned
@@ -3937,11 +4002,15 @@ static int lfs_dir_finish_removal(lfs_t *lfs, struct lfs_remove_state *p)
         return err;
     }
 
-    err = lfs_fs_pred(lfs, p->dir.m.pair, &p->cwd);
+    err = lfs_fs_pred(lfs, p->dir_head, &p->cwd);
     if (err) {
         return err;
     }
 
+    // Merge in gstate from first block splits within the directory;
+    // lfs_dir_drop will pick up the last gstate entry.
+    lfs_gstate_xor(&lfs->gdelta, &p->gstate);
+
     err = lfs_dir_drop(lfs, &p->cwd, &p->dir.m);
     if (err) {
         return err;
@@ -3952,7 +4021,7 @@ static int lfs_dir_finish_removal(lfs_t *lfs, struct lfs_remove_state *p)
 #endif
 
 #ifndef LFS_READONLY
-static int lfs_remove_(lfs_t *lfs, const char *path) {
+static int lfs_remove_(lfs_t *lfs, const char *path, lfs_dir_prep_helper_t helper) {
     // deorphan if we haven't yet, needed at most once after poweron
     int err = lfs_fs_forceconsistency(lfs);
     if (err) {
@@ -3967,7 +4036,7 @@ static int lfs_remove_(lfs_t *lfs, const char *path) {
 
     s.dir.next = lfs->mlist;
     if (lfs_tag_type3(s.tag) == LFS_TYPE_DIR) {
-        err = lfs_dir_prep_removal(lfs, &s);
+        err = lfs_dir_prep_removal(lfs, &s, helper);
         if (err < 0) {
             return err;
         }
@@ -3994,7 +4063,8 @@ static int lfs_remove_(lfs_t *lfs, const char *path) {
 #endif
 
 #ifndef LFS_READONLY
-static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) {
+static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath,
+        lfs_dir_prep_helper_t helper) {
     // deorphan if we haven't yet, needed at most once after poweron
     int err = lfs_fs_forceconsistency(lfs);
     if (err) {
@@ -4049,7 +4119,7 @@ static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) {
         // we're renaming to ourselves??
         return 0;
     } else if (lfs_tag_type3(n.tag) == LFS_TYPE_DIR) {
-        err = lfs_dir_prep_removal(lfs, &n);
+        err = lfs_dir_prep_removal(lfs, &n, helper);
         if (err) {
             return err;
         }
@@ -6019,14 +6089,32 @@ int lfs_remove(lfs_t *lfs, const char *path) {
     }
     LFS_TRACE("lfs_remove(%p, \"%s\")", (void*)lfs, path);
 
-    err = lfs_remove_(lfs, path);
+    err = lfs_remove_(lfs, path, NULL);
 
     LFS_TRACE("lfs_remove -> %d", err);
     LFS_UNLOCK(lfs->cfg);
     return err;
 }
 #endif
 
+#ifndef LFS_READONLY
+int lfs_atomic_remove(lfs_t *lfs, const char *path) {
+    int err = LFS_LOCK(lfs->cfg);
+    if (err) {
+        return err;
+    }
+    LFS_TRACE("lfs_atomic_remove(%p, \"%s\")", (void*)lfs, path);
+
+    // Note: We pass in a helper pointer here so that this extra
+    //       logic can be dropped if it is never referenced
+    err = lfs_remove_(lfs, path, lfs_dir_prep_remove_nonempty_folders);
+
+    LFS_TRACE("lfs_atomic_remove -> %d", err);
+    LFS_UNLOCK(lfs->cfg);
+    return err;
+}
+#endif
+
 #ifndef LFS_READONLY
 int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
     int err = LFS_LOCK(lfs->cfg);
@@ -6035,14 +6123,32 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
     }
     LFS_TRACE("lfs_rename(%p, \"%s\", \"%s\")", (void*)lfs, oldpath, newpath);
 
-    err = lfs_rename_(lfs, oldpath, newpath);
+    err = lfs_rename_(lfs, oldpath, newpath, NULL);
 
     LFS_TRACE("lfs_rename -> %d", err);
     LFS_UNLOCK(lfs->cfg);
     return err;
 }
 #endif
 
+#ifndef LFS_READONLY
+int lfs_atomic_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
+    int err = LFS_LOCK(lfs->cfg);
+    if (err) {
+        return err;
+    }
+    LFS_TRACE("lfs_atomic_rename(%p, \"%s\", \"%s\")", (void*)lfs, oldpath, newpath);
+
+    // Note: We pass in a helper pointer here so that this extra
+    //       logic can be dropped if it is never referenced
+    err = lfs_rename_(lfs, oldpath, newpath, lfs_dir_prep_remove_nonempty_folders);
+
+    LFS_TRACE("lfs_atomic_rename -> %d", err);
+    LFS_UNLOCK(lfs->cfg);
+    return err;
+}
+#endif
+
 int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info) {
     int err = LFS_LOCK(lfs->cfg);
     if (err) {

lfs.h

@@ -509,6 +509,17 @@ int lfs_unmount(lfs_t *lfs);
 int lfs_remove(lfs_t *lfs, const char *path);
 #endif
 
+#ifndef LFS_READONLY
+// Removes a file or directory
+//
+// If removing a directory, the directory must not have
+// any directories but it may contain files.  This is
+// non-POSIX behavior, and thus is a different call
+// than lfs_remove(...)
+// Returns a negative error code on failure.
+int lfs_atomic_remove(lfs_t *lfs, const char *path);
+#endif
+
 #ifndef LFS_READONLY
 // Rename or move a file or directory
 //
@@ -519,6 +530,19 @@ int lfs_remove(lfs_t *lfs, const char *path);
 int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath);
 #endif
 
+#ifndef LFS_READONLY
+// Rename or move a file or directory
+//
+// If the destination exists, it must match the source in type.
+// If the destination is a directory, it may not contain
+// any directories but it may contain files.  This is
+// non-POSIX behavior, and thus is a different call
+// than lfs_rename(...)
+//
+// Returns a negative error code on failure.
+int lfs_atomic_rename(lfs_t *lfs, const char *oldpath, const char *newpath);
+#endif
+
 // Find info about a file or directory
 //
 // Fills out the info structure, based on the specified file or directory.

tests/test_atomics.toml

@@ -0,0 +1,54 @@
+[cases.atomic_remove]
+defines.N = [10, 100]
+code = '''
+    lfs_t lfs;
+    char path[1024];
+    int i;
+    lfs_format(&lfs, cfg) => 0;
+    lfs_mount(&lfs, cfg) => 0;
+    lfs_ssize_t fs_size = lfs_fs_size(&lfs);
+    lfs_mkdir(&lfs, "prickly-pear") => 0;
+    srand(1);
+
+    for (i = 0; i < N; i++) {
+        lfs_file_t file;
+        sprintf(path, "prickly-pear/cactus%03d", i);
+        lfs_file_open(&lfs, &file, path,
+                LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
+        lfs_file_truncate(&lfs, &file, 512) => 0;
+        lfs_file_close(&lfs, &file) => 0;
+    }
+
+    lfs_dir_t dir;
+    struct lfs_info info;
+    lfs_dir_open(&lfs, &dir, "prickly-pear") => 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, "..") == 0);
+    for (i = 0; i < N; i++) {
+        sprintf(path, "cactus%03d", i);
+        lfs_dir_read(&lfs, &dir, &info) => 1;
+        assert(info.type == LFS_TYPE_REG);
+        assert(strcmp(info.name, path) == 0);
+    }
+    lfs_dir_read(&lfs, &dir, &info) => 0;
+    lfs_dir_close(&lfs, &dir) => 0;
+    lfs_unmount(&lfs);
+
+    lfs_mount(&lfs, cfg) => 0;
+    lfs_remove(&lfs, "prickly-pear") => LFS_ERR_NOTEMPTY;
+    lfs_atomic_remove(&lfs, "prickly-pear") => 0;
+
+    lfs_fs_size(&lfs) => fs_size;
+
+    lfs_remove(&lfs, "prickly-pear") => LFS_ERR_NOENT;
+    lfs_unmount(&lfs) => 0;
+
+    lfs_mount(&lfs, cfg) => 0;
+    lfs_fs_size(&lfs) => fs_size;
+    lfs_remove(&lfs, "prickly-pear") => LFS_ERR_NOENT;
+    lfs_unmount(&lfs) => 0;
+'''