sys::sendfile adding solaris' sendfilev wrapper proposal.

[devnexen]

No description provided.

[asomers]

This still doesn't capture anything about fd's lifetime. With this API, it's possible to use a RawFd after the file has been closed. That's why I suggested using BorrowedFd. You might be able to do it, without additional runtime cost, like this:

pub struct SendfileVec<'a> {
    raw: libc::sendfilevec_t,
    phantom: PhantomData<BorrowedFd<'a>>
}
pub fn new<F: AsFd, 'a>(fd: &'a F, off: off_t, len: usize) -> Self<'a> {
    let bfd = fd.as_fd();
    Self{raw: libc::sendfilevec_t{sfv_fd: bfd.as_raw_fd(), ...}, phantom: PhantomData}}
}

[SteveLauC]

We should probably use BorrowedFd<'a> here so that it is consistent with FdSet and PollFd

BTW, I think we should also do this to Epoll

[asomers]

Nice! It looks a lot more efficient now.

[asomers]

Constants like this should be defined in the libc crate, not in Nix.

[devnexen]

Yes I know I just did a PR for it tough :)

[SteveLauC]

We should probably use BorrowedFd<'a> here so that it is consistent with FdSet and PollFd

BTW, I think we should also do this to Epoll

[SteveLauC]

Nit: it would be great if we rename this lifetime parameter to 'fd given that libc::sendfilevec_t.sfv_fd is a file descriptor

[devnexen]

We should probably use BorrowedFd<'a> here so that it is consistent with FdSet and PollFd

BTW, I think we should also do this to Epoll

You re seing an old version.

[SteveLauC]

Emmm, why did you revert the definition of SendfileVec, for that BorrowedFd, I was just suggesting changing the function parameter type to BorrowedFd<'fd>

[SteveLauC]

Just like what we did to FdSet:

nix/src/sys/select.rs

Lines 18 to 47 in 39ad47b

	pub struct FdSet<'fd> {
	set: libc::fd_set,
	_fd: std::marker::PhantomData<BorrowedFd<'fd>>,
	}
	fn assert_fd_valid(fd: RawFd) {
	assert!(
	usize::try_from(fd).map_or(false, |fd| fd < FD_SETSIZE),
	"fd must be in the range 0..FD_SETSIZE",
	);
	}
	impl<'fd> FdSet<'fd> {
	/// Create an empty `FdSet`
	pub fn new() -> FdSet<'fd> {
	let mut fdset = mem::MaybeUninit::uninit();
	unsafe {
	libc::FD_ZERO(fdset.as_mut_ptr());
	Self {
	set: fdset.assume_init(),
	_fd: std::marker::PhantomData,
	}
	}
	}
	/// Add a file descriptor to an `FdSet`
	pub fn insert(&mut self, fd: BorrowedFd<'fd>) {
	assert_fd_valid(fd.as_raw_fd());
	unsafe { libc::FD_SET(fd.as_raw_fd(), &mut self.set) };
	}

[SteveLauC]

Add label W-blocking-upstream cause it is blocked by libc#3452

[devnexen]

libc#3452

Do we need to tough ? I can just use directly the value if that s a problem.

[SteveLauC]

Do we need to tough ?

Could you please elaborate on this? I don't think I fully understand this sentence

[devnexen]

Do we need to tough ?

Could you please elaborate on this? I don't think I fully understand this sentence

I meant blocking because of libc.

[SteveLauC]

Do we need to tough ?

Could you please elaborate on this? I don't think I fully understand this sentence

I meant blocking because of libc.

Nix does not define these constants ourselves, we add them to libc, and wait for a release of libc with the patch included, then we merge the Nix PR

[devnexen]

Do we need to tough ?

Could you please elaborate on this? I don't think I fully understand this sentence

I meant blocking because of libc.

Nix does not define these constants ourselves, we add them to libc, and wait for a release of libc with the patch included, then we merge the Nix PR

I no longer define this constant but just the value directly for the time being. but if you insist on waiting the next libc crate release, fine by me.

[SteveLauC]

I no longer define this constant but just the value directly for the time being. but if you insist on waiting the next libc crate release, fine by me.

Yes, we should wait for the libc release rather than using a magic number here

[SteveLauC]

Hi @devnexen, We now use the libc dependency from git, so you use the libc constant in this PR

[SteveLauC]

Suggested change

	/// initialises SendfileVec to send data from `fd`.
	pub fn new<F: AsFd>(
	fd: &'fd F,
	off: off_t,
	len: usize
	) -> SendfileVec<'fd> {
	let bfd = fd.as_fd();
	Self{raw: libc::sendfilevec_t{sfv_fd: bfd.as_raw_fd(), sfv_flag: 0, sfv_off:off, sfv_len: len}, phantom: PhantomData}
	}
	/// initialises SendfileVec to send data from `fd`.
	pub fn new(
	fd: BorrowedFd<'fd>,
	off: off_t,
	len: usize
	) -> SendfileVec<'fd> {
	let bfd = fd.as_fd();
	Self{raw: libc::sendfilevec_t{sfv_fd: fd.as_raw_fd(), sfv_flag: 0, sfv_off:off, sfv_len: len}, phantom: PhantomData}
	}

We should use a BorrowedFd<'fd> here to be consistent with our other interfaces

[devnexen]

Hi @devnexen, We now use the libc dependency from git, so you use the libc constant in this PR

hmmm ok sure.