more precise RawDirEntry lifetime

[gmorer]

The lifetime returned by RawDir::next() is the RawDir's lifetime and not the buffer it uses ('buf). The file name lives inside the buffer provided to the kernel.

Example

fn test<'buf, 'fd>(fd: BorrowedFd<'fd>, buf: &'buf mut Vec<u8>) -> RawDirEntry<'buf> {
    RawDir::new(fd, buf.spare_capacity_mut())
        .next()
        .unwrap()
        .unwrap()
}

Without the change, the borrow checker will tell the following error:

error: lifetime may not live long enough
  --> brr-lib/src/fs/dir.rs:40:5
   |
   |   fn test<'buf, 'fd>(fd: BorrowedFd<'fd>, buf: &'buf mut Vec<u8>) -> RawDirEntry<'buf> {
   |           -----  --- lifetime `'fd` defined here
   |           |
   |           lifetime `'buf` defined here
   | /     RawDir::new(fd, buf.spare_capacity_mut())
   | |         .next()
   | |         .unwrap()
   | |         .unwrap()
   | |_________________^ function was supposed to return data with lifetime `'buf` but it is returning data with lifetime `'fd`
   |
   = help: consider adding the following bound: `'fd: 'buf`

error[E0515]: cannot return value referencing temporary value
  --> brr-lib/src/fs/dir.rs:40:5
   |
   |       RawDir::new(fd, buf.spare_capacity_mut())
   |       ^----------------------------------------
   |       |
   |  _____temporary value created here
   | |
   | |         .next()
   | |         .unwrap()
   | |         .unwrap()
   | |_________________^ returns a value referencing data owned by the current function

The borrow checker passes with the change

[sunfishcode]

Thanks!

[SUPERCILEX]

No, this is UB. Please revert.

Here is a test that should be added to a compile_fail doc so people can't make this mistake again:

fn main() {
    let mut buf = [MaybeUninit::uninit(); 47];
    let fd = openat(CWD, c".", OFlags::DIRECTORY, Mode::empty()).unwrap();
    let mut iter = RawDir::new(fd, &mut buf);
    let item1 = iter.next().unwrap();
    let item2 = iter.next().unwrap();
    println!("{item2:?}");
    println!("{item1:?}");
}

If you're curious the output is

Ok(RawDirEntry { file_name: "rust-toolchain", file_type: RegularFile, ino: 6298992, next_entry_cookie: 2444057967821692793 })
Ok(RawDirEntry { file_name: "r", file_type: Directory, ino: 6313693, next_entry_cookie: 2212258482520683792 })

because the buffer was overwritten. With a buffer size of 1K the output is

Ok(RawDirEntry { file_name: "rust-toolchain", file_type: RegularFile, ino: 6298992, next_entry_cookie: 2444057967821692793 })
Ok(RawDirEntry { file_name: ".", file_type: Directory, ino: 6313693, next_entry_cookie: 2212258482520683792 })

[gmorer]

A yes, indeed, my bad, should be reverted.

I will open an issue to discuss the use of getdents outside of the iterator, to have more control on when the syscall is used (where the method takes a mut ref to buf so this situation can be caught by the borrow checker).

[sunfishcode]

I've now filed #1564 to revert this PR. I didn't add the testcase above because at a few brief tries I was unable to reproduce the failure. It appears dependent on the contents of the directory.

[SUPERCILEX]

@sunfishcode the test case isn't supposed to compile, so it'd be put in a doctest.

[sunfishcode]

It did compile, for me.

[SUPERCILEX]

Wat. Lemme play with it for a sec.

[SUPERCILEX]

I'm confused, it works fine? #1566

[gmorer]

I could recreate the issue, with @SUPERCILEX example, changed a bit the buffer size (smaller) and reading 3 entries, it did override the first entries.