Backwards-compatibly rewrite getdents to use special syscall.

src/error.rs

@@ -1,3 +1,5 @@
+use alloc::boxed::Box;
+
 use crate::{header::errno::STR_ERROR, platform::types::c_int};
 
 /// Positive error codes (EINVAL, not -EINVAL).
@@ -52,3 +54,28 @@ impl<T: From<i8>> ResultExt<T> for Result<T, Errno> {
         }
     }
 }
+pub trait ResultExtPtrMut<T> {
+    fn or_errno_null_mut(self) -> *mut T;
+}
+impl<T> ResultExtPtrMut<T> for Result<*mut T, Errno> {
+    fn or_errno_null_mut(self) -> *mut T {
+        match self {
+            Self::Ok(ptr) => ptr,
+            Self::Err(Errno(errno)) => {
+                crate::platform::ERRNO.set(errno);
+                core::ptr::null_mut()
+            }
+        }
+    }
+}
+impl<T> ResultExtPtrMut<T> for Result<Box<T>, Errno> {
+    fn or_errno_null_mut(self) -> *mut T {
+        match self {
+            Self::Ok(ptr) => Box::into_raw(ptr),
+            Self::Err(Errno(errno)) => {
+                crate::platform::ERRNO.set(errno);
+                core::ptr::null_mut()
+            }
+        }
+    }
+}

src/header/dirent/mod.rs

@@ -1,29 +1,114 @@
 //! dirent implementation following http://pubs.opengroup.org/onlinepubs/009695399/basedefs/dirent.h.html
 
-use alloc::boxed::Box;
+use alloc::{boxed::Box, vec::Vec};
 use core::{mem, ptr};
 
 use crate::{
     c_str::CStr,
     c_vec::CVec,
+    error::{Errno, ResultExtPtrMut},
     fs::File,
-    header::{errno, fcntl, stdlib, string},
-    io::{Seek, SeekFrom},
+    header::{fcntl, stdlib, string},
     platform::{self, types::*, Pal, Sys},
 };
 
-const DIR_BUF_SIZE: usize = mem::size_of::<dirent>() * 3;
+use super::errno::{EINVAL, EIO, ENOMEM};
 
-// No repr(C) needed, C won't see the content
+const INITIAL_BUFSIZE: usize = 512;
+
+// No repr(C) needed, as this is a completely opaque struct. Being accessed as a pointer, in C it's
+// just defined as `struct DIR`.
 pub struct DIR {
     file: File,
-    buf: [c_char; DIR_BUF_SIZE],
-    // index and len are specified in bytes
-    index: usize,
-    len: usize,
+    buf: Vec<u8>,
+    buf_offset: usize,
 
     // The last value of d_off, used by telldir
-    offset: usize,
+    opaque_offset: u64,
+}
+impl DIR {
+    pub fn new(path: CStr) -> Result<Box<Self>, Errno> {
+        Ok(Box::new(Self {
+            file: File::open(
+                path,
+                fcntl::O_RDONLY | fcntl::O_DIRECTORY | fcntl::O_CLOEXEC,
+            )?,
+            buf: Vec::with_capacity(INITIAL_BUFSIZE),
+            buf_offset: 0,
+            opaque_offset: 0,
+        }))
+    }
+    fn next_dirent(&mut self) -> Result<*mut dirent, Errno> {
+        let mut this_dent = self.buf.get(self.buf_offset..).ok_or(Errno(EIO))?;
+        if this_dent.is_empty() {
+            let size = loop {
+                self.buf.resize(self.buf.capacity(), 0_u8);
+                // TODO: uninitialized memory?
+                match Sys::getdents(*self.file, &mut self.buf, self.opaque_offset) {
+                    Ok(size) => break size,
+                    Err(Errno(EINVAL)) => {
+                        self.buf
+                            .try_reserve_exact(self.buf.len())
+                            .map_err(|_| Errno(ENOMEM))?;
+                        continue;
+                    }
+                    Err(Errno(other)) => return Err(Errno(other)),
+                }
+            };
+            self.buf.truncate(size);
+            self.buf_offset = 0;
+
+            if size == 0 {
+                return Ok(core::ptr::null_mut());
+            }
+            this_dent = &self.buf;
+        }
+        let (this_reclen, this_next_opaque) =
+            unsafe { Sys::dent_reclen_offset(this_dent, self.buf_offset).ok_or(Errno(EIO))? };
+
+        //println!("CDENT {} {}+{}", self.opaque_offset, self.buf_offset, this_reclen);
+
+        let next_off = self
+            .buf_offset
+            .checked_add(usize::from(this_reclen))
+            .ok_or(Errno(EIO))?;
+        if next_off > self.buf.len() {
+            return Err(Errno(EIO));
+        }
+        if this_dent.len() < usize::from(this_reclen) {
+            // Don't want memory corruption if a scheme is adversarial.
+            return Err(Errno(EIO));
+        }
+        let dent_ptr = this_dent.as_ptr() as *mut dirent;
+
+        self.opaque_offset = this_next_opaque;
+        self.buf_offset = next_off;
+        Ok(dent_ptr)
+    }
+    fn seek(&mut self, off: u64) {
+        let Ok(_) = Sys::dir_seek(*self.file, off) else {
+            return;
+        };
+        self.buf.clear();
+        self.buf_offset = 0;
+        self.opaque_offset = off;
+    }
+    fn rewind(&mut self) {
+        self.opaque_offset = 0;
+        let Ok(_) = Sys::dir_seek(*self.file, 0) else {
+            return;
+        };
+        self.buf.clear();
+        self.buf_offset = 0;
+        self.opaque_offset = 0;
+    }
+    fn close(mut self) -> c_int {
+        // Reference files aren't closed when dropped
+        self.file.reference = true;
+
+        // TODO: result
+        Sys::close(*self.file)
+    }
 }
 
 #[repr(C)]
@@ -36,68 +121,50 @@ pub struct dirent {
     pub d_name: [c_char; 256],
 }
 
+#[cfg(target_os = "redox")]
+const _: () = {
+    use core::mem::{offset_of, size_of};
+    use syscall::dirent::DirentHeader;
+
+    if offset_of!(dirent, d_ino) != offset_of!(DirentHeader, inode) {
+        panic!("struct dirent layout mismatch (inode)");
+    }
+    if offset_of!(dirent, d_off) != offset_of!(DirentHeader, next_opaque_id) {
+        panic!("struct dirent layout mismatch (inode)");
+    }
+    if offset_of!(dirent, d_reclen) != offset_of!(DirentHeader, record_len) {
+        panic!("struct dirent layout mismatch (len)");
+    }
+    if offset_of!(dirent, d_type) != offset_of!(DirentHeader, kind) {
+        panic!("struct dirent layout mismatch (kind)");
+    }
+    if offset_of!(dirent, d_name) != size_of::<DirentHeader>() {
+        panic!("struct dirent layout mismatch (name)");
+    }
+};
+
 #[no_mangle]
 pub unsafe extern "C" fn opendir(path: *const c_char) -> *mut DIR {
     let path = CStr::from_ptr(path);
-    let file = match File::open(
-        path,
-        fcntl::O_RDONLY | fcntl::O_DIRECTORY | fcntl::O_CLOEXEC,
-    ) {
-        Ok(file) => file,
-        Err(_) => return ptr::null_mut(),
-    };
 
-    Box::into_raw(Box::new(DIR {
-        file,
-        buf: [0; DIR_BUF_SIZE],
-        index: 0,
-        len: 0,
-        offset: 0,
-    }))
+    DIR::new(path).or_errno_null_mut()
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn closedir(dir: *mut DIR) -> c_int {
-    let mut dir = Box::from_raw(dir);
-
-    let ret = Sys::close(*dir.file);
-
-    // Reference files aren't closed when dropped
-    dir.file.reference = true;
-
-    ret
+pub extern "C" fn closedir(dir: Box<DIR>) -> c_int {
+    dir.close()
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dirfd(dir: *mut DIR) -> c_int {
-    *((*dir).file)
+pub extern "C" fn dirfd(dir: &mut DIR) -> c_int {
+    *dir.file
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn readdir(dir: *mut DIR) -> *mut dirent {
-    if (*dir).index >= (*dir).len {
-        let read = Sys::getdents(
-            *(*dir).file,
-            (*dir).buf.as_mut_ptr() as *mut dirent,
-            (*dir).buf.len(),
-        );
-        if read <= 0 {
-            if read != 0 && read != -errno::ENOENT {
-                platform::ERRNO.set(-read);
-            }
-            return ptr::null_mut();
-        }
-
-        (*dir).index = 0;
-        (*dir).len = read as usize;
-    }
-
-    let ptr = (*dir).buf.as_mut_ptr().add((*dir).index) as *mut dirent;
-
-    (*dir).offset = (*ptr).d_off as usize;
-    (*dir).index += (*ptr).d_reclen as usize;
-    ptr
+pub extern "C" fn readdir(dir: &mut DIR) -> *mut dirent {
+    dir.next_dirent().or_errno_null_mut()
 }
+
 // #[no_mangle]
 pub extern "C" fn readdir_r(
     _dir: *mut DIR,
@@ -108,19 +175,19 @@ pub extern "C" fn readdir_r(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn telldir(dir: *mut DIR) -> c_long {
-    (*dir).offset as c_long
+pub extern "C" fn telldir(dir: &mut DIR) -> c_long {
+    dir.opaque_offset as c_long
 }
 #[no_mangle]
-pub unsafe extern "C" fn seekdir(dir: *mut DIR, off: c_long) {
-    let _ = (*dir).file.seek(SeekFrom::Start(off as u64));
-    (*dir).offset = off as usize;
-    (*dir).index = 0;
-    (*dir).len = 0;
+pub extern "C" fn seekdir(dir: &mut DIR, off: c_long) {
+    dir.seek(
+        off.try_into()
+            .expect("off must come from telldir, thus never negative"),
+    );
 }
 #[no_mangle]
-pub unsafe extern "C" fn rewinddir(dir: *mut DIR) {
-    seekdir(dir, 0)
+pub extern "C" fn rewinddir(dir: &mut DIR) {
+    dir.rewind();
 }
 
 #[no_mangle]
@@ -149,7 +216,7 @@ pub unsafe extern "C" fn scandir(
     platform::ERRNO.set(0);
 
     loop {
-        let entry: *mut dirent = readdir(dir);
+        let entry: *mut dirent = readdir(&mut *dir);
         if entry.is_null() {
             break;
         }
@@ -170,7 +237,7 @@ pub unsafe extern "C" fn scandir(
         }
     }
 
-    closedir(dir);
+    closedir(Box::from_raw(dir));
 
     let len = vec.len();
     if let Err(_) = vec.shrink_to_fit() {

src/header/fcntl/cbindgen.toml

@@ -6,9 +6,5 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true

src/header/signal/cbindgen.toml

@@ -6,10 +6,6 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true
 

src/header/stdlib/cbindgen.toml

@@ -6,9 +6,6 @@ style = "Type"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_pointer_width=64" = "__LP64__"
-
 [enum]
 prefix_with_name = true
 

src/header/sys_epoll/cbindgen.toml

@@ -5,10 +5,5 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-"target_pointer_width=64" = "__LP64__"
-
 [enum]
 prefix_with_name = true

src/header/sys_ioctl/cbindgen.toml

@@ -4,10 +4,6 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true
 

src/header/sys_mman/cbindgen.toml

@@ -6,9 +6,5 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true

src/header/sys_resource/cbindgen.toml

@@ -6,10 +6,6 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true
 

src/header/sys_socket/cbindgen.toml

@@ -6,10 +6,6 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true
 

src/header/termios/cbindgen.toml

@@ -5,10 +5,6 @@ style = "Tag"
 no_includes = true
 cpp_compat = true
 
-[defines]
-"target_os=linux" = "__linux__"
-"target_os=redox" = "__redox__"
-
 [enum]
 prefix_with_name = true
 

src/header/time/cbindgen.toml

@@ -7,7 +7,3 @@ cpp_compat = true
 
 [enum]
 prefix_with_name = true
-
-[defines]
-"target_os = linux" = "__linux__"
-"target_os = redox" = "__redox__"

src/header/unistd/cbindgen.toml

@@ -8,7 +8,3 @@ cpp_compat = true
 
 [enum]
 prefix_with_name = true
-
-[defines]
-"target_os = linux" = "__linux__"
-"target_os = redox" = "__redox__"

src/lib.rs

@@ -12,6 +12,7 @@
 #![feature(maybe_uninit_slice)]
 #![feature(maybe_uninit_uninit_array)]
 #![feature(lang_items)]
+#![feature(let_chains)]
 #![feature(linkage)]
 #![feature(stmt_expr_attributes)]
 #![feature(str_internals)]

src/platform/linux/mod.rs

@@ -1,5 +1,5 @@
 use crate::io::Write;
-use core::{arch::asm, ptr};
+use core::{arch::asm, mem::offset_of, ptr};
 
 use super::{types::*, Pal, ERRNO};
 use crate::{
@@ -12,6 +12,7 @@ use crate::header::{
     sys_stat::stat,
     sys_statvfs::statvfs,
     sys_time::{timeval, timezone},
+    unistd::SEEK_SET,
 };
 // use header::sys_times::tms;
 use crate::{
@@ -281,8 +282,16 @@ impl Pal for Sys {
         }
     }
 
-    fn getdents(fd: c_int, dirents: *mut dirent, bytes: usize) -> c_int {
-        unsafe { syscall!(GETDENTS64, fd, dirents, bytes) as c_int }
+    fn getdents(fd: c_int, buf: &mut [u8], _off: u64) -> Result<usize, Errno> {
+        e_raw(unsafe { syscall!(GETDENTS64, fd, buf.as_mut_ptr(), buf.len()) })
+    }
+    fn dir_seek(fd: c_int, off: u64) -> Result<(), Errno> {
+        e_raw(unsafe { syscall!(LSEEK, fd, off, SEEK_SET) })?;
+        Ok(())
+    }
+    unsafe fn dent_reclen_offset(this_dent: &[u8], offset: usize) -> Option<(u16, u64)> {
+        let dent = this_dent.as_ptr().cast::<dirent>();
+        Some(((*dent).d_reclen, (*dent).d_off as u64))
     }
 
     fn getegid() -> gid_t {

src/platform/pal/mod.rs

@@ -93,7 +93,13 @@ pub trait Pal {
 
     fn getcwd(buf: *mut c_char, size: size_t) -> *mut c_char;
 
-    fn getdents(fd: c_int, dirents: *mut dirent, bytes: usize) -> c_int;
+    fn getdents(fd: c_int, buf: &mut [u8], opaque_offset: u64) -> Result<usize, Errno>;
+    fn dir_seek(fd: c_int, opaque_offset: u64) -> Result<(), Errno>;
+
+    // SAFETY: This_dent must satisfy platform-specific size and alignment constraints. On Linux,
+    // this means the buffer came from a valid getdents64 invocation, whereas on Redox, every
+    // possible this_dent slice is safe (and will be validated).
+    unsafe fn dent_reclen_offset(this_dent: &[u8], offset: usize) -> Option<(u16, u64)>;
 
     fn getegid() -> gid_t;
 

src/platform/redox/mod.rs

@@ -1,8 +1,13 @@
-use core::{convert::TryFrom, mem, ptr, slice, str};
+use core::{
+    convert::TryFrom,
+    mem::{self, size_of},
+    ptr, slice, str,
+};
 use redox_rt::RtTcb;
 use syscall::{
     self,
     data::{Map, Stat as redox_stat, StatVfs as redox_statvfs, TimeSpec as redox_timespec},
+    dirent::{DirentHeader, DirentKind},
     Error, PtraceEvent, Result, EMFILE,
 };
 
@@ -12,7 +17,10 @@ use crate::{
     fs::File,
     header::{
         dirent::dirent,
-        errno::{EBADF, EBADFD, EBADR, EINVAL, EIO, ENOENT, ENOMEM, ENOSYS, EPERM, ERANGE},
+        errno::{
+            EBADF, EBADFD, EBADR, EINVAL, EIO, ENAMETOOLONG, ENOENT, ENOMEM, ENOSYS, EOPNOTSUPP,
+            EPERM, ERANGE,
+        },
         fcntl, limits,
         sys_mman::{MAP_ANONYMOUS, MAP_FAILED, PROT_READ, PROT_WRITE},
         sys_random,
@@ -353,97 +361,74 @@ impl Pal for Sys {
         buf
     }
 
-    fn getdents(fd: c_int, mut dirents: *mut dirent, max_bytes: usize) -> c_int {
-        //TODO: rewrite this code. Originally the *dirents = dirent { ... } stuff below caused
-        // massive issues. This has been hacked around, but it still isn't perfect
+    fn getdents(fd: c_int, buf: &mut [u8], opaque: u64) -> Result<usize, Errno> {
+        //println!("GETDENTS {} into ({:p}+{})", fd, buf.as_ptr(), buf.len());
 
-        // Get initial reading position
-        let mut read = match syscall::lseek(fd as usize, 0, syscall::SEEK_CUR) {
-            Ok(pos) => pos as isize,
-            Err(err) => return -err.errno,
-        };
+        const HEADER_SIZE: usize = size_of::<DirentHeader>();
 
-        let mut written = 0;
-        let mut buf = [0; 1024];
-
-        let mut name = [0; 256];
-        let mut i = 0;
-
-        let mut flush = |written: &mut usize, i: &mut usize, name: &mut [c_char; 256]| {
-            if *i < name.len() {
-                // Set NUL byte
-                name[*i] = 0;
-            }
-            // Get size: full size - unused bytes
-            if *written + mem::size_of::<dirent>() > max_bytes {
-                // Seek back to after last read entry and return
-                match syscall::lseek(fd as usize, read, syscall::SEEK_SET) {
-                    Ok(_) => return Some(*written as c_int),
-                    Err(err) => return Some(-err.errno),
-                }
-            }
-            let size = mem::size_of::<dirent>() - name.len().saturating_sub(*i + 1);
-            unsafe {
-                //This is the offending code mentioned above
-                *dirents = dirent {
-                    d_ino: 0,
-                    d_off: read as off_t,
-                    d_reclen: size as c_ushort,
-                    d_type: 0,
-                    d_name: *name,
-                };
-                dirents = (dirents as *mut u8).offset(size as isize) as *mut dirent;
-            }
-            read += *i as isize + /* newline */ 1;
-            *written += size;
-            *i = 0;
-            None
-        };
-
-        loop {
-            // Read a chunk from the directory
-            let len = match syscall::read(fd as usize, &mut buf) {
-                Ok(0) => {
-                    if i > 0 {
-                        if let Some(value) = flush(&mut written, &mut i, &mut name) {
-                            return value;
-                        }
-                    }
-                    return written as c_int;
-                }
-                Ok(n) => n,
-                Err(err) => return -err.errno,
-            };
-
-            // Handle everything
-            let mut start = 0;
-            while start < len {
-                let buf = &buf[start..len];
-
-                // Copy everything up until a newline
-                let newline = buf.iter().position(|&c| c == b'\n');
-                let pre_len = newline.unwrap_or(buf.len());
-                let post_len = newline.map(|i| i + 1).unwrap_or(buf.len());
-                if i < pre_len {
-                    // Reserve space for NUL byte
-                    let name_len = name.len() - 1;
-                    let name = &mut name[i..name_len];
-                    let copy = pre_len.min(name.len());
-                    let buf = unsafe { slice::from_raw_parts(buf.as_ptr() as *const c_char, copy) };
-                    name[..copy].copy_from_slice(buf);
-                }
-
-                i += pre_len;
-                start += post_len;
-
-                // Write the directory entry
-                if newline.is_some() {
-                    if let Some(value) = flush(&mut written, &mut i, &mut name) {
-                        return value;
-                    }
-                }
+        // Use syscall if it exists.
+        match unsafe {
+            syscall::syscall5(
+                syscall::SYS_GETDENTS,
+                fd as usize,
+                buf.as_mut_ptr() as usize,
+                buf.len(),
+                HEADER_SIZE,
+                opaque as usize,
+            )
+        } {
+            Err(Error {
+                errno: EOPNOTSUPP | ENOSYS,
+            }) => (),
+            other => {
+                //println!("REAL GETDENTS {:?}", other);
+                return Ok(other?);
             }
         }
+
+        // Otherwise, for legacy schemes, assume the buffer is pre-arranged (all schemes do this in
+        // practice), and just read the name. If multiple names appear, pretend it didn't happen
+        // and just use the first entry.
+
+        let (header, name) = buf.split_at_mut(size_of::<DirentHeader>());
+
+        let bytes_read = Sys::pread(fd, name, opaque as i64)? as usize;
+        if bytes_read == 0 {
+            return Ok(0);
+        }
+
+        let (name_len, advance) = match name[..bytes_read].iter().position(|c| *c == b'\n') {
+            Some(idx) => (idx, idx + 1),
+
+            // Insufficient space for NUL byte, or entire entry was not read. Indicate we need a
+            // larger buffer.
+            None if bytes_read == name.len() => return Err(Errno(EINVAL)),
+
+            None => (bytes_read, name.len()),
+        };
+        name[name_len] = b'\0';
+
+        let record_len = u16::try_from(size_of::<DirentHeader>() + name_len + 1)
+            .map_err(|_| Error::new(ENAMETOOLONG))?;
+        header.copy_from_slice(&DirentHeader {
+            inode: 0,
+            next_opaque_id: opaque + advance as u64,
+            record_len,
+            kind: DirentKind::Unspecified as u8,
+        });
+        //println!("EMULATED GETDENTS");
+
+        Ok(record_len.into())
+    }
+    fn dir_seek(_fd: c_int, _off: u64) -> Result<(), Errno> {
+        // Redox getdents takes an explicit (opaque) offset, so this is a no-op.
+        Ok(())
+    }
+    // NOTE: fn is unsafe, but this just means we can assume more things. impl is safe
+    unsafe fn dent_reclen_offset(this_dent: &[u8], offset: usize) -> Option<(u16, u64)> {
+        let mut header = DirentHeader::default();
+        header.copy_from_slice(&this_dent.get(..size_of::<DirentHeader>())?);
+        Some((header.record_len, header.next_opaque_id))
     }
 
     fn getegid() -> gid_t {