Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for dirent related function #558

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#739](https://github.com/nix-rust/nix/pull/739))
- Added nix::sys::ptrace::detach.
([#749](https://github.com/nix-rust/nix/pull/749))
- Added `nix::dirent::{opendir, fdopendir, readdir, telldir, seekdir}`
([#558](https://github.com/nix-rust/nix/pull/558))

### Changed
- Renamed existing `ptrace` wrappers to encourage namespacing ([#692](https://github.com/nix-rust/nix/pull/692))
Expand Down
135 changes: 135 additions & 0 deletions src/dirent.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
//! Directory Stream functions
//!
//! [Further reading and details on the C API](http://man7.org/linux/man-pages/man3/opendir.3.html)

use {Result, Error, Errno, NixPath};
use errno;
use libc::{self, DIR, c_long};
use std::convert::{AsRef, Into};
use std::ffi::CStr;
use std::mem;

#[cfg(any(target_os = "linux"))]
use libc::ino64_t;

#[cfg(any(target_os = "android"))]
use libc::ino_t as ino64_t;

#[cfg(any(target_os = "linux", target_os = "android"))]
use libc::{dirent64, readdir64};

#[cfg(not(any(target_os = "linux", target_os = "android")))]
use libc::{dirent as dirent64, ino_t as ino64_t, readdir as readdir64};

#[cfg(not(any(target_os = "ios", target_os = "macos")))]
use std::os::unix::io::RawFd;

/// Directory Stream object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we call this DirectoryStream or DirStream instead of Dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also renamed Dirent to DirectoryEntry for consistency.

pub struct DirectoryStream(*mut DIR);

impl AsRef<DIR> for DirectoryStream {
fn as_ref(&self) -> &DIR {
unsafe { &*self.0 }
}
}

/// Consumes directory stream and return underlying directory pointer.
///
/// The pointer must be deallocated manually using `libc::closedir`
impl Into<*mut DIR> for DirectoryStream {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A convention in std is to use a method named into_raw here when you're unwrapping a type. I'd suggest using that here instead of Into<*mut DIR>. I don't see this used anywhere, however. Can you either remove it if it's unnecessary, or if it's necessary add it to the test or use it in an example.

fn into(self) -> *mut DIR {
let dirp = self.0;
mem::forget(self);
dirp
}
}

impl Drop for DirectoryStream {
fn drop(&mut self) {
unsafe { libc::closedir(self.0) };
}
}

/// A directory entry
pub struct DirectoryEntry<'a>(&'a dirent64);

impl<'a> DirectoryEntry<'a> {
/// File name
pub fn name(&self) -> &CStr {
unsafe{
CStr::from_ptr(self.0.d_name.as_ptr())
}
}

/// Inode number
pub fn inode(&self) -> ino64_t {
#[cfg(not(any(target_os = "freebsd", target_os = "netbsd", target_os="dragonfly")))]
return self.0.d_ino;
#[cfg(any(target_os = "freebsd", target_os = "netbsd", target_os="dragonfly"))]
return self.0.d_fileno;
}
}

impl<'a> AsRef<dirent64> for DirectoryEntry<'a> {
fn as_ref(&self) -> &dirent64 {
self.0
}
}

/// Opens a directory stream corresponding to the directory name.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you run cargo doc --no-deps --open locally with this code and make sure the first sentence shows up nicely on the page listing all the functions here? I'm certain that some of these sentences are too long and that you need an empty /// on the next line to make them show up nicely.

///
/// The stream is positioned at the first entry in the directory.
pub fn opendir<P: ?Sized + NixPath>(name: &P) -> Result<DirectoryStream> {
let dirp = try!(name.with_nix_path(|cstr| unsafe { libc::opendir(cstr.as_ptr()) }));
if dirp.is_null() {
Err(Error::last().into())
} else {
Ok(DirectoryStream(dirp))
}
}

/// Returns directory stream corresponding to the open file descriptor `fd`
///
/// After a successful call to this function, `fd` is used internally by
/// the implementation, and should not otherwise be used by the application
#[cfg(not(any(target_os = "ios", target_os = "macos")))]
pub fn fdopendir(fd: RawFd) -> Result<DirectoryStream> {
let dirp = unsafe { libc::fdopendir(fd) };
if dirp.is_null() {
Err(Error::last().into())
} else {
Ok(DirectoryStream(dirp))
}
}

/// Returns the next directory entry in the directory stream.
///
/// It returns `Some(None)` on reaching the end of the directory stream.
pub fn readdir<'a>(dir: &'a mut DirectoryStream) -> Result<Option<DirectoryEntry>> {
let dirent = unsafe {
Errno::clear();
readdir64(dir.0)
};
if dirent.is_null() {
match Errno::last() {
errno::UnknownErrno => Ok(None),
_ => Err(Error::last().into()),
}
} else {
Ok(Some(DirectoryEntry(unsafe { &*dirent })))
}
}

/// Sets the location in the directory stream from which the next `readdir` call will start.
///
/// The `loc` argument should be a value returned by a previous call to `telldir`
#[cfg(not(any(target_os = "android")))]
pub fn seekdir<'a>(dir: &'a mut DirectoryStream, loc: c_long) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking a newtype around c_long for use as the return value of telldir and the 2nd argument of seekdir would be good, since that's an invariant that should be embedded in the type system. I can't think of a good name for it, hopefully you can, but I think we should definitely do it.

unsafe { libc::seekdir(dir.0, loc) };
}

/// Returns the current location associated with the directory stream.
#[cfg(not(any(target_os = "android")))]
pub fn telldir<'a>(dir: &'a mut DirectoryStream) -> c_long {
unsafe { libc::telldir(dir.0) }
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub extern crate libc;

pub use errno::Errno;

pub mod dirent;
pub mod errno;
pub mod features;
pub mod fcntl;
Expand Down
1 change: 1 addition & 0 deletions test/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ extern crate tempfile;
extern crate nix_test as nixtest;

mod sys;
mod test_dirent;
mod test_fcntl;
#[cfg(target_os = "linux")]
mod test_mq;
Expand Down
50 changes: 50 additions & 0 deletions test/test_dirent.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use libc;
use nix::dirent::{self, opendir, readdir, seekdir, telldir, DirectoryStream};
use std::path::Path;
use tempdir::TempDir;

fn test_readdir<Open>(open_fn: Open)
where Open: Fn(&Path) -> DirectoryStream
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check the full name of the directory? It's confusing that you're checking just the first character of the filename. It'd also be helpful if you created a directory in the tempdir you made so you have 3 directories, then you iterate through all of them checking their names, and then seek back to check that seeking works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarantee on the order of directory entries.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it's worth checking their names, we can treat it as a set, where the order doesn't matter but the contents should still be the same. We'd expect to encounter both "." and "..", but we aren't certain of the order, but that's fine, we can still do that check.

let tempdir = TempDir::new("nix-test_readdir")
.unwrap_or_else(|e| panic!("tempdir failed: {}", e));
let mut dir = open_fn(tempdir.path());
let first_inode = readdir(&mut dir)
.unwrap()
.unwrap()
.inode();

let pos = telldir(&mut dir);

let second_inode = readdir(&mut dir)
.unwrap()
.unwrap()
.inode();
seekdir(&mut dir, pos);

let second_inode_again = readdir(&mut dir)
.unwrap()
.unwrap()
.inode();

assert_ne!(first_inode, second_inode);
assert_eq!(second_inode, second_inode_again);

// end of directory
assert!(readdir(&mut dir).unwrap().is_none());

unsafe { libc::closedir(dir.into()) };
}

#[test]
fn test_opendir() {
test_readdir(|path| opendir(path).unwrap());
}

#[cfg(not(any(target_os = "ios", target_os = "macos")))]
#[test]
fn test_fdopendir() {
use std::os::unix::io::IntoRawFd;
use std::fs::File;
test_readdir(|path| dirent::fdopendir(File::open(path).unwrap().into_raw_fd()).unwrap());
}