From 97e2412fb4719fb46f25300387af4e2d99c786bd Mon Sep 17 00:00:00 2001 From: "carpentry-heartbeat[bot]" Date: Mon, 20 Jul 2026 12:38:33 +0200 Subject: [PATCH] fix(walk): propagate errors from unreadable subdirectories walk-recur discarded the Result of its own recursive call, so a subdirectory that could not be opened (permissions, a race with an unlink, a symlink loop) left the walk reporting Success after visiting only part of the tree. Bind the recursive result and, on Error, set res and break -- the same shape the stat-failure path in the same cond already uses, so Dir.close still runs and the handle is not leaked. Semantics: the walk aborts at the first subdirectory it cannot descend into and returns that error, consistent with the existing stat behaviour. Documented on walk and walk-with. Discarding the result also miscompiled the recursive call: Carp emits self-recursion through a Lambda whose callback is cast to a void-returning pointer when the value is unused. On ABIs that return Result via a hidden sret pointer (32-bit ARM), that cast shifts every argument, so recursion silently did nothing. Binding the result emits a correctly typed call. Adds a regression test that chmods an inner directory to 000 and asserts walk returns Error, plus a check that a fully readable tree still succeeds and visits every file. The test probes whether the directory is really unreadable rather than assuming, so it skips instead of passing vacuously as root, and restores the mode before cleaning up. --- file.carp | 31 ++++++++++++++++++++----------- test/file.carp | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/file.carp b/file.carp index 8cb5cfa..790492a 100644 --- a/file.carp +++ b/file.carp @@ -126,7 +126,10 @@ You can also ask about the modes of the file, using the functions `readable?`, (is-dir lres) (do (when @(WalkOptions.recursive? spec) - (walk-recur &f op spec)) + (let [rec (walk-recur &f op spec)] + (when-do (Result.error? &rec) + (set! res rec) + (break)))) (when @(WalkOptions.match-dirs? spec) (~op &f))) (~op &f)))))) (Dir.close dir) @@ -146,23 +149,29 @@ or dotfiles.") (doc walk-with "walks a directory with a custom [walk mode](#walk-mode). -Returns a result containing either nothing, or an error if the directory -couldn’t be opened due to permission errors or if the user tried to open a -nonexistant directory.") +Returns a result containing either nothing, or an error if a directory couldn’t +be opened due to permission errors or if the user tried to open a nonexistant +directory. + +The walk aborts at the first subdirectory it can’t descend into and returns that +error, so a success means the entire tree was visited.") (defn walk-with [s callback options] (walk-recur s callback options)) (doc walk "walks a directory with the default walk mode (see also [`default-walk`](#default-walk)). -Returns a result containing either nothing, or an error if the directory -couldn’t be opened due to permission errors or if the user tried to open a -nonexistant directory.") +Returns a result containing either nothing, or an error if a directory couldn’t +be opened due to permission errors or if the user tried to open a nonexistant +directory. + +The walk aborts at the first subdirectory it can’t descend into and returns that +error, so a success means the entire tree was visited.") (defn walk [s callback] (walk-with s callback &default-walk)) (doc map-with "walks a directory with a custom [walk mode](#walk-mode). Returns a result containing either the results of the walker function as an -array or an error if the directory couldn’t be opened due to permission errors +array or an error if a directory couldn’t be opened due to permission errors or if the user tried to open a nonexistant directory.") (defn map-with [s callback options] (let [res [] @@ -175,13 +184,13 @@ or if the user tried to open a nonexistant directory.") [`default-walk`](#default-walk)). Returns a result containing either the results of the walker function as an -array or an error if the directory couldn’t be opened due to permission errors +array or an error if a directory couldn’t be opened due to permission errors or if the user tried to open a nonexistant directory.") (defn map [s callback] (map-with s callback &default-walk)) (doc contents-with "walks a directory with a custom [walk mode](#walk-mode). -Returns a result containing either the directory contents or an error if the +Returns a result containing either the directory contents or an error if a directory couldn’t be opened due to permission errors or if the user tried to open a nonexistant directory.") (defn contents-with [s options] (map-with s © options)) @@ -190,7 +199,7 @@ open a nonexistant directory.") [`default-walk`](#default-walk)). Returns a result containing either the contents of that directory or an error -if the directory couldn’t be opened due to permission errors or if the user +if a directory couldn’t be opened due to permission errors or if the user tried to open a nonexistant directory.") (defn contents [s] (contents-with s &default-walk)) diff --git a/test/file.carp b/test/file.carp index 550b110..ce32d2e 100644 --- a/test/file.carp +++ b/test/file.carp @@ -5,6 +5,24 @@ (def test-acc 0) +(defn- clean-walk-fixture [] + (do + (ignore (System.system "chmod -R u+rwx walk-fixture 2> /dev/null")) + (ignore (System.system "rm -rf walk-fixture")))) + +(defn- make-walk-fixture [] + (do + (clean-walk-fixture) + (ignore (System.system "mkdir -p walk-fixture/readable walk-fixture/locked")) + (ignore + (System.system + "touch walk-fixture/top walk-fixture/readable/inner walk-fixture/locked/deep")))) + +; chmod 000 does not stop root, so probe the directory instead of assuming +(defn- locked-dir-unreadable? [] + (let [d (Dir.open (cstr "walk-fixture/locked"))] + (if (null? d) true (do (Dir.close d) false)))) + (defn main [] (with-test test (assert-equal test @@ -88,6 +106,24 @@ &(Array.sorted &[@"test/nested/fixture" @"test/file.carp"]) &(Array.sorted &(Result.from-success (File.contents "test") [])) "contents works on directory") + (do (make-walk-fixture) @test) + (assert-equal test + &(Array.sorted + &[@"walk-fixture/locked/deep" + @"walk-fixture/readable/inner" + @"walk-fixture/top"]) + &(Array.sorted &(Result.from-success (File.contents "walk-fixture") [])) + "walk visits every file of a readable tree") + (assert-true test + (do + (ignore (System.system "chmod 000 walk-fixture/locked")) + (if (locked-dir-unreadable?) + (error? &(File.walk "walk-fixture" &(fn [_] ()))) + (do + (IO.println "\t(skipped, unreadable directories are not enforced)") + true))) + "walk reports an error for an unreadable subdirectory") + (do (clean-walk-fixture) @test) (let-do [f (unsafe-from-success (File.open "example"))] (File.remove &f) @test)))