Skip to content

Commit 45b82b8

Browse files
committed
fix(ls): properly release parent nodes in DFS
Fixes a regression introduced in #11386 (sorry) where DFS nodes did not properly free the recorded inodes of the parent directories, causing critical errors in some instances of recursive symlinks.
1 parent c6a57bb commit 45b82b8

2 files changed

Lines changed: 37 additions & 19 deletions

File tree

src/uu/ls/src/ls.rs

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,11 @@ impl Colorable for PathData<'_> {
974974
}
975975
}
976976

977-
type DirData = (PathBuf, bool);
977+
struct LsNode {
978+
path: PathBuf,
979+
needs_blank_line: bool,
980+
fi: FileInformation,
981+
}
978982

979983
// A struct to encapsulate state that is passed around from `list` functions.
980984
#[cfg_attr(not(unix), allow(dead_code))]
@@ -995,7 +999,7 @@ struct ListState<'a> {
995999
#[cfg(not(unix))]
9961000
gid_cache: (),
9971001
recent_time_range: RangeInclusive<SystemTime>,
998-
stack: Vec<DirData>,
1002+
stack: Vec<LsNode>,
9991003
listed_ancestors: FxHashSet<FileInformation>,
10001004
initial_locs_len: usize,
10011005
display_buf: Vec<u8>,
@@ -1109,22 +1113,19 @@ pub fn list(locs: Vec<&Path>, config: &Config) -> UResult<()> {
11091113
)?;
11101114

11111115
// Only runs if it must list recursively.
1112-
while let Some(dir_data) = state.stack.pop() {
1113-
let read_dir = match fs::read_dir(&dir_data.0) {
1116+
while let Some(node) = state.stack.pop() {
1117+
let read_dir = match fs::read_dir(&node.path) {
11141118
Err(err) => {
11151119
// flush stdout buffer before the error to preserve formatting and order
11161120
state.out.flush()?;
1117-
show!(LsError::IOErrorContext(
1118-
path_data.path().to_path_buf(),
1119-
err,
1120-
path_data.command_line
1121-
));
1121+
show!(LsError::IOErrorContext(node.path, err, false));
11221122
continue;
11231123
}
11241124
Ok(rd) => rd,
11251125
};
11261126

1127-
depth_first_list(dir_data, read_dir, config, &mut state, &mut dired, false)?;
1127+
state.listed_ancestors.remove(&node.fi);
1128+
depth_first_list(node.into(), read_dir, config, &mut state, &mut dired, false)?;
11281129

11291130
// Heuristic to ensure stack does not keep its capacity forever if there is
11301131
// combinatorial explosion; we decrease it logarithmically here.
@@ -1208,14 +1209,14 @@ fn sort_entries(entries: &mut [PathData], config: &Config) {
12081209
}
12091210

12101211
fn depth_first_list(
1211-
(dir_path, needs_blank_line): DirData,
1212+
(dir_path, needs_blank_line): (PathBuf, bool),
12121213
mut read_dir: ReadDir,
12131214
config: &Config,
12141215
state: &mut ListState,
12151216
dired: &mut DiredOutput,
12161217
is_top_level: bool,
12171218
) -> UResult<()> {
1218-
let path_data = PathData::new(dir_path.as_path().into(), None, None, config, false);
1219+
let path_data = PathData::new(dir_path.into(), None, None, config, false);
12191220

12201221
// Print dir heading - name... 'total' comes after error display
12211222
if state.initial_locs_len > 1 || config.recursive {
@@ -1253,7 +1254,7 @@ fn depth_first_list(
12531254
}
12541255

12551256
// Append entries with initial dot files and record their existence
1256-
let (ref mut buf, trim) = if config.files == Files::All {
1257+
let (mut buf, trim) = if config.files == Files::All {
12571258
const DOT_DIRECTORIES: usize = 2;
12581259
let v = vec![
12591260
PathData::new(
@@ -1299,20 +1300,20 @@ fn depth_first_list(
12991300
// Relinquish unused space since we won't need it anymore.
13001301
buf.shrink_to_fit();
13011302

1302-
sort_entries(buf, config);
1303+
sort_entries(&mut buf, config);
13031304

13041305
if config.format == Format::Long || config.alloc_size {
1305-
let total = write_total(buf, config, &mut state.out)?;
1306+
let total = write_total(&buf, config, &mut state.out)?;
13061307
if config.dired {
13071308
dired::add_total(dired, total);
13081309
}
13091310
}
13101311

1311-
display_items(buf, config, state, dired)?;
1312+
display_items(&buf, config, state, dired)?;
13121313

13131314
if config.recursive {
13141315
for e in buf
1315-
.iter()
1316+
.into_iter()
13161317
.skip(trim)
13171318
.filter(|p| p.file_type().is_some_and(FileType::is_dir))
13181319
.rev()
@@ -1327,13 +1328,13 @@ fn depth_first_list(
13271328
));
13281329
} else {
13291330
let fi = FileInformation::from_path(e.path(), e.must_dereference)?;
1330-
if state.listed_ancestors.insert(fi) {
1331+
if state.listed_ancestors.insert(fi.clone()) {
13311332
// Push to stack, but with a less aggressive growth curve.
13321333
let (cap, len) = (state.stack.capacity(), state.stack.len());
13331334
if cap == len {
13341335
state.stack.reserve_exact(len / 4 + 4);
13351336
}
1336-
state.stack.push((e.path().to_path_buf(), true));
1337+
state.stack.push(LsNode::new(e.p_buf, needs_blank_line, fi));
13371338
} else {
13381339
state.out.flush()?;
13391340
show!(LsError::AlreadyListedError(e.path().to_path_buf()));
@@ -1344,6 +1345,22 @@ fn depth_first_list(
13441345
Ok(())
13451346
}
13461347

1348+
impl LsNode {
1349+
fn new(path: impl Into<PathBuf>, needs_blank_line: bool, fi: FileInformation) -> Self {
1350+
Self {
1351+
path: path.into(),
1352+
needs_blank_line,
1353+
fi,
1354+
}
1355+
}
1356+
}
1357+
1358+
impl From<LsNode> for (PathBuf, bool) {
1359+
fn from(val: LsNode) -> Self {
1360+
(val.path, val.needs_blank_line)
1361+
}
1362+
}
1363+
13471364
fn get_metadata_with_deref_opt(p_buf: &Path, dereference: bool) -> std::io::Result<Metadata> {
13481365
if dereference {
13491366
p_buf.metadata()

src/uucore/src/lib/features/fs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ macro_rules! has {
4646
}
4747

4848
/// Information to uniquely identify a file
49+
#[derive(Clone)]
4950
pub struct FileInformation(
5051
#[cfg(unix)] nix::sys::stat::FileStat,
5152
#[cfg(windows)] winapi_util::file::Information,

0 commit comments

Comments
 (0)