Skip to content

Commit

Permalink
Root::create_file: return File not Handle for O_CREAT
Browse files Browse the repository at this point in the history
Since we let the user specify the open flags a-la Handle::reopen, it
makes far more sense to return a File from Root::create_file than a
Handle.

The bindings also need to be updated to match these semantics.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
  • Loading branch information
cyphar committed Oct 8, 2024
1 parent 59a151c commit 77a3d34
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 25 deletions.
3 changes: 1 addition & 2 deletions contrib/bindings/python/pathrs/_pathrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,7 @@ def creat(self, path, filemode, mode="r", extra_flags=0):
fd = libpathrs_so.pathrs_creat(self.fileno(), path, flags, filemode)
if fd < 0:
raise Error._fetch(fd) or INTERNAL_ERROR
# TODO: This should actually return an os.fdopen.
return Handle(fd)
return os.fdopen(fd, mode)

# TODO: creat_raw?

Expand Down
10 changes: 3 additions & 7 deletions go-pathrs/root_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,17 @@ func (r *Root) ResolveNoFollow(path string) (*Handle, error) {
// Create creates a file within the Root's directory tree at the given path,
// and returns a handle to the file. The provided mode is used for the new file
// (the process's umask applies).
func (r *Root) Create(path string, flags int, mode os.FileMode) (*Handle, error) {
func (r *Root) Create(path string, flags int, mode os.FileMode) (*os.File, error) {
unixMode, err := toUnixMode(mode)
if err != nil {
return nil, err
}
return withFileFd(r.inner, func(rootFd uintptr) (*Handle, error) {
return withFileFd(r.inner, func(rootFd uintptr) (*os.File, error) {
handleFd, err := pathrsCreat(rootFd, path, flags, unixMode)
if err != nil {
return nil, err
}
handleFile, err := mkFile(uintptr(handleFd))
if err != nil {
return nil, err
}
return &Handle{inner: handleFile}, nil
return mkFile(uintptr(handleFd))
})
}

Expand Down
11 changes: 10 additions & 1 deletion src/capi/ret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@

use crate::{capi::error as capi_error, error::Error, Handle, Root};

use std::os::unix::io::{IntoRawFd, OwnedFd};
use std::{
fs::File,
os::unix::io::{IntoRawFd, OwnedFd},
};

use libc::c_int;

Expand Down Expand Up @@ -64,6 +67,12 @@ impl IntoCReturn for Handle {
}
}

impl IntoCReturn for File {
fn into_c_return(self) -> CReturn {
OwnedFd::from(self).into_c_return()
}
}

impl<V> IntoCReturn for Result<V, Error>
where
V: IntoCReturn,
Expand Down
8 changes: 4 additions & 4 deletions src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
};

use std::{
fs::Permissions,
fs::{File, Permissions},
io::Error as IOError,
os::unix::{
ffi::OsStrExt,
Expand Down Expand Up @@ -367,7 +367,7 @@ impl Root {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Handle, Error> {
) -> Result<File, Error> {
self.as_ref().create_file(path, flags, perm)
}

Expand Down Expand Up @@ -829,7 +829,7 @@ impl RootRef<'_> {
path: P,
mut flags: OpenFlags,
perm: &Permissions,
) -> Result<Handle, Error> {
) -> Result<File, Error> {
// The path doesn't exist yet, so we need to get a safe reference to the
// parent and just operate on the final (slashless) component.
let (dir, name) = self
Expand All @@ -851,7 +851,7 @@ impl RootRef<'_> {
}
})?;

Ok(Handle::from_fd_unchecked(fd))
Ok(fd.into())
}

/// Within the [`RootRef`]'s tree, create a directory and any of its parent
Expand Down
10 changes: 5 additions & 5 deletions src/tests/capi/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::{
};

use std::{
fs::Permissions,
fs::{File, Permissions},
os::unix::{
fs::PermissionsExt,
io::{AsFd, BorrowedFd, OwnedFd},
Expand Down Expand Up @@ -140,14 +140,14 @@ impl CapiRoot {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<CapiHandle, CapiError> {
) -> Result<File, CapiError> {
let root_fd = self.inner.as_fd();
let path = capi_utils::path_to_cstring(path);

capi_utils::call_capi_fd(|| unsafe {
capi::core::pathrs_creat(root_fd.into(), path.as_ptr(), flags.bits(), perm.mode())
})
.map(CapiHandle::from_fd_unchecked)
.map(File::from)
}

fn mkdir_all<P: AsRef<Path>>(
Expand Down Expand Up @@ -269,7 +269,7 @@ impl RootImpl for CapiRoot {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error> {
) -> Result<File, Self::Error> {
self.create_file(path, flags, perm)
}

Expand Down Expand Up @@ -348,7 +348,7 @@ impl RootImpl for &CapiRoot {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error> {
) -> Result<File, Self::Error> {
CapiRoot::create_file(self, path, flags, perm)
}

Expand Down
12 changes: 6 additions & 6 deletions src/tests/traits/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
};

use std::{
fs::Permissions,
fs::{File, Permissions},
os::unix::io::{AsFd, OwnedFd},
path::{Path, PathBuf},
};
Expand Down Expand Up @@ -57,7 +57,7 @@ pub(in crate::tests) trait RootImpl: AsFd + std::fmt::Debug + Sized {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error>;
) -> Result<File, Self::Error>;

fn mkdir_all<P: AsRef<Path>>(
&self,
Expand Down Expand Up @@ -122,7 +122,7 @@ impl RootImpl for Root {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error> {
) -> Result<File, Self::Error> {
self.create_file(path, flags, perm)
}

Expand Down Expand Up @@ -199,7 +199,7 @@ impl RootImpl for &Root {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error> {
) -> Result<File, Self::Error> {
Root::create_file(self, path, flags, perm)
}

Expand Down Expand Up @@ -276,7 +276,7 @@ impl RootImpl for RootRef<'_> {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error> {
) -> Result<File, Self::Error> {
self.create_file(path, flags, perm)
}

Expand Down Expand Up @@ -353,7 +353,7 @@ impl RootImpl for &RootRef<'_> {
path: P,
flags: OpenFlags,
perm: &Permissions,
) -> Result<Self::Handle, Self::Error> {
) -> Result<File, Self::Error> {
RootRef::create_file(self, path, flags, perm)
}

Expand Down

0 comments on commit 77a3d34

Please sign in to comment.