Support removal of non-empty folders via lfs_remove_recursive(...)

Introduce a function that removes a folder and all of its child folders
recursively.

Author: tim-nordell-nimbelink

lfs.c

@@ -3903,11 +3903,7 @@ static int lfs_dir_prep_remove_nonempty_folders(lfs_t *lfs, struct lfs_remove_st
     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.
+    // tags.
     while(true) {
         if (p->dir.m.count == id) {
             if (!p->dir.m.split) {
@@ -3935,7 +3931,11 @@ static int lfs_dir_prep_remove_nonempty_folders(lfs_t *lfs, struct lfs_remove_st
         }
 
         if (lfs_tag_type3(tag) == LFS_TYPE_DIR) {
-            // We're not allowed to find leafs
+            // In case we are finding leafs, we need to update our tag, too,
+            // so that the upper routine knows where this folder was in the
+            // parent folder.  The parent routine will decide if we need
+            // to return LFS_ERR_NOTEMPTY all the way back out.
+            p->tag = tag;
             return LFS_ERR_NOTEMPTY;
         }
 
@@ -3947,10 +3947,15 @@ static int lfs_dir_prep_remove_nonempty_folders(lfs_t *lfs, struct lfs_remove_st
 #endif
 
 #ifndef LFS_READONLY
+// Note: This returns 1 when it had to descend to find a leaf node,
+//       0 when it didn't have to descend.
 static int lfs_dir_prep_removal(lfs_t *lfs, struct lfs_remove_state *p,
-        lfs_dir_prep_helper_t helper) {
+        lfs_dir_prep_helper_t helper, bool find_leaf) {
+    lfs_block_t depth = 0;
+    lfs_stag_t res;
 
-    lfs_stag_t res = lfs_dir_get(lfs, &p->cwd, LFS_MKTAG(0x700, 0x3ff, 0),
+new_leaf:
+    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) {
         return (int)res;
@@ -3972,7 +3977,20 @@ static int lfs_dir_prep_removal(lfs_t *lfs, struct lfs_remove_state *p,
         }
 
         err = helper(lfs, p);
-        if (err) {
+        if (err == LFS_ERR_NOTEMPTY && find_leaf) {
+            if (depth++ > lfs->cfg->block_count / 2) {
+                return LFS_ERR_CORRUPT;
+            }
+
+            // Descend CWD
+            memcpy(&p->cwd, &p->dir.m, sizeof(p->cwd));
+
+            // And loop
+            goto new_leaf;
+        }
+
+        if (err)
+        {
             return err;
         }
     }
@@ -3989,7 +4007,7 @@ static int lfs_dir_prep_removal(lfs_t *lfs, struct lfs_remove_state *p,
     p->dir.id = 0;
     lfs->mlist = &p->dir;
 
-    return 0;
+    return depth != 0;
 }
 #endif
 
@@ -4021,42 +4039,61 @@ 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, lfs_dir_prep_helper_t helper) {
+static int lfs_remove_(lfs_t *lfs, const char *path, lfs_dir_prep_helper_t helper, bool recursive) {
+    // For the recursive flag to work, we must have the helper defined.
+    if (recursive && NULL == helper) {
+        return LFS_ERR_INVAL;
+    }
+
     // deorphan if we haven't yet, needed at most once after poweron
     int err = lfs_fs_forceconsistency(lfs);
     if (err) {
         return err;
     }
 
+    bool at_toplevel = true;
     struct lfs_remove_state s;
-    s.tag = lfs_dir_find(lfs, &s.cwd, &path, NULL);
-    if (s.tag < 0 || lfs_tag_id(s.tag) == 0x3ff) {
-        return (s.tag < 0) ? (int)s.tag : LFS_ERR_INVAL;
-    }
+    do {
+        // Since we want to keep our in-memory usage minimal, we rely on
+        // the disk structure to "recurse".  We'll go back to the top
+        // level path every time we need to find a new leaf.  Technically
+        // we could remember the top-level path state so we don't have
+        // to search again for this folder entry...
+        at_toplevel = true;
+        s.tag = lfs_dir_find(lfs, &s.cwd, &path, NULL);
+        if (s.tag < 0 || lfs_tag_id(s.tag) == 0x3ff) {
+            return (s.tag < 0) ? (int)s.tag : LFS_ERR_INVAL;
+        }
+
+        s.dir.next = lfs->mlist;
+        if (lfs_tag_type3(s.tag) == LFS_TYPE_DIR) {
+            err = lfs_dir_prep_removal(lfs, &s, helper, recursive);
+            if (err == 1) {
+                // We had to switch folders and we're no longer in the top-level folder.
+                // This means we need to go back from the top after we finish the
+                // removal process.
+                at_toplevel = false;
+            } else if (err < 0) {
+                return err;
+            }
+        }
 
-    s.dir.next = lfs->mlist;
-    if (lfs_tag_type3(s.tag) == LFS_TYPE_DIR) {
-        err = lfs_dir_prep_removal(lfs, &s, helper);
-        if (err < 0) {
+        // delete the entry
+        err = lfs_dir_commit(lfs, &s.cwd, LFS_MKATTRS(
+                {LFS_MKTAG(LFS_TYPE_DELETE, lfs_tag_id(s.tag), 0), NULL}));
+        if (err) {
+            lfs->mlist = s.dir.next;
             return err;
         }
-    }
 
-    // delete the entry
-    err = lfs_dir_commit(lfs, &s.cwd, LFS_MKATTRS(
-            {LFS_MKTAG(LFS_TYPE_DELETE, lfs_tag_id(s.tag), 0), NULL}));
-    if (err) {
         lfs->mlist = s.dir.next;
-        return err;
-    }
-
-    lfs->mlist = s.dir.next;
-    if (lfs_tag_type3(s.tag) == LFS_TYPE_DIR) {
-        err = lfs_dir_finish_removal(lfs, &s);
-        if (err) {
-            return err;
+        if (lfs_tag_type3(s.tag) == LFS_TYPE_DIR) {
+            err = lfs_dir_finish_removal(lfs, &s);
+            if (err) {
+                return err;
+            }
         }
-    }
+    } while(!at_toplevel);
 
     return 0;
 }
@@ -4119,7 +4156,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, helper);
+        err = lfs_dir_prep_removal(lfs, &n, helper, false);
         if (err) {
             return err;
         }
@@ -6089,14 +6126,30 @@ int lfs_remove(lfs_t *lfs, const char *path) {
     }
     LFS_TRACE("lfs_remove(%p, \"%s\")", (void*)lfs, path);
 
-    err = lfs_remove_(lfs, path, NULL);
+    err = lfs_remove_(lfs, path, NULL, false);
 
     LFS_TRACE("lfs_remove -> %d", err);
     LFS_UNLOCK(lfs->cfg);
     return err;
 }
 #endif
 
+#ifndef LFS_READONLY
+int lfs_remove_recursive(lfs_t *lfs, const char *path) {
+    int err = LFS_LOCK(lfs->cfg);
+    if (err) {
+        return err;
+    }
+    LFS_TRACE("lfs_remove_recursive(%p, \"%s\")", (void*)lfs, path);
+
+    err = lfs_remove_(lfs, path, lfs_dir_prep_remove_nonempty_folders, true);
+
+    LFS_TRACE("lfs_remove_recursive -> %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);
@@ -6107,7 +6160,7 @@ int lfs_atomic_remove(lfs_t *lfs, const char *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);
+    err = lfs_remove_(lfs, path, lfs_dir_prep_remove_nonempty_folders, false);
 
     LFS_TRACE("lfs_atomic_remove -> %d", err);
     LFS_UNLOCK(lfs->cfg);

lfs.h

@@ -509,6 +509,18 @@ int lfs_unmount(lfs_t *lfs);
 int lfs_remove(lfs_t *lfs, const char *path);
 #endif
 
+#ifndef LFS_READONLY
+// Removes a file or directory recursively
+//
+// Note: If power is interrupted, you might end up with
+//       a partially emptied folder.  This routine removes
+//       a leaf at a time atomically and will eventually
+//       remove the only leaf left (the top leaf).
+//
+// Returns a negative error code on failure.
+int lfs_remove_recursive(lfs_t *lfs, const char *path);
+#endif
+
 #ifndef LFS_READONLY
 // Removes a file or directory
 //

tests/test_remove_recursive.toml

@@ -0,0 +1,67 @@
+[cases.recursive_remove]
+defines.N = [1, 3, 10, 100]
+defines.M = [1, 3, 10, 100]
+if = 'M * N < BLOCK_COUNT/2'
+code = '''
+    lfs_t lfs;
+    char path[1024];
+    int i, j;
+    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++) {
+        sprintf(path, "prickly-pear/cactus%03d", i);
+        lfs_mkdir(&lfs, path) => 0;
+    }
+
+    // For the first 5 directories, insert child files
+    for (i = 0; i < 5 && i < N; i++) {
+        lfs_file_t file;
+        for (j = 0; j < M; j++) {
+            sprintf(path, "prickly-pear/cactus%03d/%03d", i, j);
+            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_DIR);
+        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_remove_recursive(&lfs, "prickly-pear") => 0;
+
+    lfs_fs_size(&lfs) => fs_size;
+
+    lfs_remove(&lfs, "prickly-pear/cactus001") => LFS_ERR_NOENT;
+
+    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;
+'''