Skip to content

Commit 6f0ca03

Browse files
bors[bot]asomers
bors[bot]
and
asomers
authored
Merge #1244
1244: Clippy cleanup r=asomers a=asomers Reported-by: Clippy Co-authored-by: Alan Somers <asomers@gmail.com>
2 parents 679c786 + 27d39b2 commit 6f0ca03

File tree

11 files changed

+186
-59
lines changed

11 files changed

+186
-59
lines changed

‎CHANGELOG.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
3535
(#[1252](https://github.com/nix-rust/nix/pull/1252))
3636
- Added support for `Ipv4PacketInfo` and `Ipv6PacketInfo` to `ControlMessage`.
3737
(#[1222](https://github.com/nix-rust/nix/pull/1222))
38+
- `CpuSet` and `UnixCredentials` now implement `Default`.
39+
(#[1244](https://github.com/nix-rust/nix/pull/1244))
3840

3941
### Changed
4042
- Changed `fallocate` return type from `c_int` to `()` (#[1201](https://github.com/nix-rust/nix/pull/1201))
@@ -44,6 +46,15 @@ This project adheres to [Semantic Versioning](http://semver.org/).
4446
(#[1245](https://github.com/nix-rust/nix/pull/1245))
4547
- `execv`, `execve`, `execvp` and `execveat` in `::nix::unistd` and `reboot` in
4648
`::nix::sys::reboot` now return `Result<Infallible>` instead of `Result<Void>` (#[1239](https://github.com/nix-rust/nix/pull/1239))
49+
- `sys::socket::sockaddr_storage_to_addr` is no longer `unsafe`. So is
50+
`offset_of!`.
51+
- `sys::socket::sockaddr_storage_to_addr`, `offset_of!`, and `Errno::clear` are
52+
no longer `unsafe`.
53+
- `SockAddr::as_ffi_pair`,`sys::socket::sockaddr_storage_to_addr`, `offset_of!`,
54+
and `Errno::clear` are no longer `unsafe`.
55+
(#[1244](https://github.com/nix-rust/nix/pull/1244))
56+
- Several `Inotify` methods now take `self` by value instead of by reference
57+
(#[1244](https://github.com/nix-rust/nix/pull/1244))
4758

4859
### Fixed
4960

@@ -61,6 +72,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
6172
correctness's sake across all architectures and compilers, though now bugs
6273
have been reported so far.
6374
(#[1243](https://github.com/nix-rust/nix/pull/1243))
75+
- Fixed unaligned pointer read in `Inotify::read_events`.
76+
(#[1244](https://github.com/nix-rust/nix/pull/1244))
6477

6578
### Removed
6679

‎src/errno.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ cfg_if! {
4646
}
4747

4848
/// Sets the platform-specific errno to no-error
49-
unsafe fn clear() {
50-
*errno_location() = 0;
49+
fn clear() {
50+
// Safe because errno is a thread-local variable
51+
unsafe {
52+
*errno_location() = 0;
53+
}
5154
}
5255

5356
/// Returns the platform-specific value of errno
@@ -70,7 +73,7 @@ impl Errno {
7073
from_i32(err)
7174
}
7275

73-
pub unsafe fn clear() {
76+
pub fn clear() {
7477
clear()
7578
}
7679

‎src/fcntl.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ libc_bitflags!(
161161
}
162162
);
163163

164+
// The conversion is not identical on all operating systems.
165+
#[allow(clippy::identity_conversion)]
164166
pub fn open<P: ?Sized + NixPath>(path: &P, oflag: OFlag, mode: Mode) -> Result<RawFd> {
165167
let fd = path.with_nix_path(|cstr| {
166168
unsafe { libc::open(cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) }
@@ -169,6 +171,8 @@ pub fn open<P: ?Sized + NixPath>(path: &P, oflag: OFlag, mode: Mode) -> Result<R
169171
Errno::result(fd)
170172
}
171173

174+
// The conversion is not identical on all operating systems.
175+
#[allow(clippy::identity_conversion)]
172176
#[cfg(not(target_os = "redox"))]
173177
pub fn openat<P: ?Sized + NixPath>(
174178
dirfd: RawFd,

‎src/macros.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,11 @@ macro_rules! libc_enum {
208208
/// offset of `field` within struct `ty`
209209
#[cfg(not(target_os = "redox"))]
210210
macro_rules! offset_of {
211-
($ty:ty, $field:ident) => {
212-
&(*(ptr::null() as *const $ty)).$field as *const _ as usize
213-
}
211+
($ty:ty, $field:ident) => {{
212+
// Safe because we don't actually read from the dereferenced pointer
213+
#[allow(unused_unsafe)] // for when the macro is used in an unsafe block
214+
unsafe {
215+
&(*(ptr::null() as *const $ty)).$field as *const _ as usize
216+
}
217+
}}
214218
}

‎src/sched.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ mod sched_linux_like {
8080
if field >= CpuSet::count() {
8181
Err(Error::Sys(Errno::EINVAL))
8282
} else {
83-
Ok(unsafe { libc::CPU_SET(field, &mut self.cpu_set) })
83+
unsafe { libc::CPU_SET(field, &mut self.cpu_set); }
84+
Ok(())
8485
}
8586
}
8687

@@ -90,7 +91,8 @@ mod sched_linux_like {
9091
if field >= CpuSet::count() {
9192
Err(Error::Sys(Errno::EINVAL))
9293
} else {
93-
Ok(unsafe { libc::CPU_CLR(field, &mut self.cpu_set) })
94+
unsafe { libc::CPU_CLR(field, &mut self.cpu_set);}
95+
Ok(())
9496
}
9597
}
9698

@@ -100,6 +102,12 @@ mod sched_linux_like {
100102
}
101103
}
102104

105+
impl Default for CpuSet {
106+
fn default() -> Self {
107+
Self::new()
108+
}
109+
}
110+
103111
/// `sched_setaffinity` set a thread's CPU affinity mask
104112
/// ([`sched_setaffinity(2)`](http://man7.org/linux/man-pages/man2/sched_setaffinity.2.html))
105113
///
@@ -181,8 +189,8 @@ mod sched_linux_like {
181189

182190
let res = unsafe {
183191
let combined = flags.bits() | signal.unwrap_or(0);
184-
let ptr = stack.as_mut_ptr().offset(stack.len() as isize);
185-
let ptr_aligned = ptr.offset((ptr as usize % 16) as isize * -1);
192+
let ptr = stack.as_mut_ptr().add(stack.len());
193+
let ptr_aligned = ptr.sub(ptr as usize % 16);
186194
libc::clone(
187195
mem::transmute(
188196
callback as extern "C" fn(*mut Box<dyn FnMut() -> isize>) -> i32,

‎src/sys/inotify.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ use libc::{
2929
};
3030
use std::ffi::{OsString,OsStr,CStr};
3131
use std::os::unix::ffi::OsStrExt;
32-
use std::mem::size_of;
32+
use std::mem::{MaybeUninit, size_of};
3333
use std::os::unix::io::{RawFd,AsRawFd,FromRawFd};
34+
use std::ptr;
3435
use crate::unistd::read;
3536
use crate::Result;
3637
use crate::NixPath;
@@ -130,7 +131,7 @@ impl Inotify {
130131
/// Returns a watch descriptor. This is not a File Descriptor!
131132
///
132133
/// For more information see, [inotify_add_watch(2)](http://man7.org/linux/man-pages/man2/inotify_add_watch.2.html).
133-
pub fn add_watch<P: ?Sized + NixPath>(&self,
134+
pub fn add_watch<P: ?Sized + NixPath>(self,
134135
path: &P,
135136
mask: AddWatchFlags)
136137
-> Result<WatchDescriptor>
@@ -151,14 +152,14 @@ impl Inotify {
151152
///
152153
/// For more information see, [inotify_rm_watch(2)](http://man7.org/linux/man-pages/man2/inotify_rm_watch.2.html).
153154
#[cfg(target_os = "linux")]
154-
pub fn rm_watch(&self, wd: WatchDescriptor) -> Result<()> {
155+
pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()> {
155156
let res = unsafe { libc::inotify_rm_watch(self.fd, wd.wd) };
156157

157158
Errno::result(res).map(drop)
158159
}
159160

160161
#[cfg(target_os = "android")]
161-
pub fn rm_watch(&self, wd: WatchDescriptor) -> Result<()> {
162+
pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()> {
162163
let res = unsafe { libc::inotify_rm_watch(self.fd, wd.wd as u32) };
163164

164165
Errno::result(res).map(drop)
@@ -170,21 +171,24 @@ impl Inotify {
170171
///
171172
/// Returns as many events as available. If the call was non blocking and no
172173
/// events could be read then the EAGAIN error is returned.
173-
pub fn read_events(&self) -> Result<Vec<InotifyEvent>> {
174+
pub fn read_events(self) -> Result<Vec<InotifyEvent>> {
174175
let header_size = size_of::<libc::inotify_event>();
175-
let mut buffer = [0u8; 4096];
176+
const BUFSIZ: usize = 4096;
177+
let mut buffer = [0u8; BUFSIZ];
176178
let mut events = Vec::new();
177179
let mut offset = 0;
178180

179181
let nread = read(self.fd, &mut buffer)?;
180182

181183
while (nread - offset) >= header_size {
182184
let event = unsafe {
183-
&*(
184-
buffer
185-
.as_ptr()
186-
.offset(offset as isize) as *const libc::inotify_event
187-
)
185+
let mut event = MaybeUninit::<libc::inotify_event>::uninit();
186+
ptr::copy_nonoverlapping(
187+
buffer.as_ptr().add(offset),
188+
event.as_mut_ptr() as *mut u8,
189+
(BUFSIZ - offset).min(header_size)
190+
);
191+
event.assume_init()
188192
};
189193

190194
let name = match event.len {
@@ -193,7 +197,7 @@ impl Inotify {
193197
let ptr = unsafe {
194198
buffer
195199
.as_ptr()
196-
.offset(offset as isize + header_size as isize)
200+
.add(offset + header_size)
197201
as *const c_char
198202
};
199203
let cstr = unsafe { CStr::from_ptr(ptr) };

‎src/sys/mman.rs‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -260,20 +260,37 @@ libc_bitflags!{
260260
}
261261
}
262262

263-
/// Locks all memory pages that contain part of the address range with `length` bytes starting at
264-
/// `addr`. Locked pages never move to the swap area.
263+
/// Locks all memory pages that contain part of the address range with `length`
264+
/// bytes starting at `addr`.
265+
///
266+
/// Locked pages never move to the swap area.
267+
///
268+
/// # Safety
269+
///
270+
/// `addr` must meet all the requirements described in the `mlock(2)` man page.
265271
pub unsafe fn mlock(addr: *const c_void, length: size_t) -> Result<()> {
266272
Errno::result(libc::mlock(addr, length)).map(drop)
267273
}
268274

269-
/// Unlocks all memory pages that contain part of the address range with `length` bytes starting at
270-
/// `addr`.
275+
/// Unlocks all memory pages that contain part of the address range with
276+
/// `length` bytes starting at `addr`.
277+
///
278+
/// # Safety
279+
///
280+
/// `addr` must meet all the requirements described in the `munlock(2)` man
281+
/// page.
271282
pub unsafe fn munlock(addr: *const c_void, length: size_t) -> Result<()> {
272283
Errno::result(libc::munlock(addr, length)).map(drop)
273284
}
274285

275-
/// Locks all memory pages mapped into this process' address space. Locked pages never move to the
276-
/// swap area.
286+
/// Locks all memory pages mapped into this process' address space.
287+
///
288+
/// Locked pages never move to the swap area.
289+
///
290+
/// # Safety
291+
///
292+
/// `addr` must meet all the requirements described in the `mlockall(2)` man
293+
/// page.
277294
pub fn mlockall(flags: MlockAllFlags) -> Result<()> {
278295
unsafe { Errno::result(libc::mlockall(flags.bits())) }.map(drop)
279296
}
@@ -283,8 +300,11 @@ pub fn munlockall() -> Result<()> {
283300
unsafe { Errno::result(libc::munlockall()) }.map(drop)
284301
}
285302

286-
/// Calls to mmap are inherently unsafe, so they must be made in an unsafe block. Typically
287-
/// a higher-level abstraction will hide the unsafe interactions with the mmap'd region.
303+
/// allocate memory, or map files or devices into memory
304+
///
305+
/// # Safety
306+
///
307+
/// See the `mmap(2)` man page for detailed requirements.
288308
pub unsafe fn mmap(addr: *mut c_void, length: size_t, prot: ProtFlags, flags: MapFlags, fd: RawFd, offset: off_t) -> Result<*mut c_void> {
289309
let ret = libc::mmap(addr, length, prot.bits(), flags.bits(), fd, offset);
290310

@@ -295,10 +315,22 @@ pub unsafe fn mmap(addr: *mut c_void, length: size_t, prot: ProtFlags, flags: Ma
295315
}
296316
}
297317

318+
/// remove a mapping
319+
///
320+
/// # Safety
321+
///
322+
/// `addr` must meet all the requirements described in the `munmap(2)` man
323+
/// page.
298324
pub unsafe fn munmap(addr: *mut c_void, len: size_t) -> Result<()> {
299325
Errno::result(libc::munmap(addr, len)).map(drop)
300326
}
301327

328+
/// give advice about use of memory
329+
///
330+
/// # Safety
331+
///
332+
/// See the `madvise(2)` man page. Take special care when using
333+
/// `MmapAdvise::MADV_FREE`.
302334
pub unsafe fn madvise(addr: *mut c_void, length: size_t, advise: MmapAdvise) -> Result<()> {
303335
Errno::result(libc::madvise(addr, length, advise as i32)).map(drop)
304336
}
@@ -332,6 +364,12 @@ pub unsafe fn mprotect(addr: *mut c_void, length: size_t, prot: ProtFlags) -> Re
332364
Errno::result(libc::mprotect(addr, length, prot.bits())).map(drop)
333365
}
334366

367+
/// synchronize a mapped region
368+
///
369+
/// # Safety
370+
///
371+
/// `addr` must meet all the requirements described in the `msync(2)` man
372+
/// page.
335373
pub unsafe fn msync(addr: *mut c_void, length: size_t, flags: MsFlags) -> Result<()> {
336374
Errno::result(libc::msync(addr, length, flags.bits())).map(drop)
337375
}

‎src/sys/socket/addr.rs‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -768,39 +768,60 @@ impl SockAddr {
768768
/// with the size of the actual data type. sockaddr is commonly used as a proxy for
769769
/// a superclass as C doesn't support inheritance, so many functions that take
770770
/// a sockaddr * need to take the size of the underlying type as well and then internally cast it back.
771-
pub unsafe fn as_ffi_pair(&self) -> (&libc::sockaddr, libc::socklen_t) {
771+
pub fn as_ffi_pair(&self) -> (&libc::sockaddr, libc::socklen_t) {
772772
match *self {
773773
SockAddr::Inet(InetAddr::V4(ref addr)) => (
774-
&*(addr as *const libc::sockaddr_in as *const libc::sockaddr),
774+
// This cast is always allowed in C
775+
unsafe {
776+
&*(addr as *const libc::sockaddr_in as *const libc::sockaddr)
777+
},
775778
mem::size_of_val(addr) as libc::socklen_t
776779
),
777780
SockAddr::Inet(InetAddr::V6(ref addr)) => (
778-
&*(addr as *const libc::sockaddr_in6 as *const libc::sockaddr),
781+
// This cast is always allowed in C
782+
unsafe {
783+
&*(addr as *const libc::sockaddr_in6 as *const libc::sockaddr)
784+
},
779785
mem::size_of_val(addr) as libc::socklen_t
780786
),
781787
SockAddr::Unix(UnixAddr(ref addr, len)) => (
782-
&*(addr as *const libc::sockaddr_un as *const libc::sockaddr),
788+
// This cast is always allowed in C
789+
unsafe {
790+
&*(addr as *const libc::sockaddr_un as *const libc::sockaddr)
791+
},
783792
(len + offset_of!(libc::sockaddr_un, sun_path)) as libc::socklen_t
784793
),
785794
#[cfg(any(target_os = "android", target_os = "linux"))]
786795
SockAddr::Netlink(NetlinkAddr(ref sa)) => (
787-
&*(sa as *const libc::sockaddr_nl as *const libc::sockaddr),
796+
// This cast is always allowed in C
797+
unsafe {
798+
&*(sa as *const libc::sockaddr_nl as *const libc::sockaddr)
799+
},
788800
mem::size_of_val(sa) as libc::socklen_t
789801
),
790802
#[cfg(any(target_os = "android", target_os = "linux"))]
791803
SockAddr::Alg(AlgAddr(ref sa)) => (
792-
&*(sa as *const libc::sockaddr_alg as *const libc::sockaddr),
804+
// This cast is always allowed in C
805+
unsafe {
806+
&*(sa as *const libc::sockaddr_alg as *const libc::sockaddr)
807+
},
793808
mem::size_of_val(sa) as libc::socklen_t
794809
),
795810
#[cfg(any(target_os = "ios", target_os = "macos"))]
796811
SockAddr::SysControl(SysControlAddr(ref sa)) => (
797-
&*(sa as *const libc::sockaddr_ctl as *const libc::sockaddr),
812+
// This cast is always allowed in C
813+
unsafe {
814+
&*(sa as *const libc::sockaddr_ctl as *const libc::sockaddr)
815+
},
798816
mem::size_of_val(sa) as libc::socklen_t
799817

800818
),
801819
#[cfg(any(target_os = "android", target_os = "linux"))]
802820
SockAddr::Link(LinkAddr(ref addr)) => (
803-
&*(addr as *const libc::sockaddr_ll as *const libc::sockaddr),
821+
// This cast is always allowed in C
822+
unsafe {
823+
&*(addr as *const libc::sockaddr_ll as *const libc::sockaddr)
824+
},
804825
mem::size_of_val(addr) as libc::socklen_t
805826
),
806827
#[cfg(any(target_os = "dragonfly",
@@ -810,12 +831,18 @@ impl SockAddr {
810831
target_os = "netbsd",
811832
target_os = "openbsd"))]
812833
SockAddr::Link(LinkAddr(ref addr)) => (
813-
&*(addr as *const libc::sockaddr_dl as *const libc::sockaddr),
834+
// This cast is always allowed in C
835+
unsafe {
836+
&*(addr as *const libc::sockaddr_dl as *const libc::sockaddr)
837+
},
814838
mem::size_of_val(addr) as libc::socklen_t
815839
),
816840
#[cfg(target_os = "linux")]
817841
SockAddr::Vsock(VsockAddr(ref sa)) => (
818-
&*(sa as *const libc::sockaddr_vm as *const libc::sockaddr),
842+
// This cast is always allowed in C
843+
unsafe {
844+
&*(sa as *const libc::sockaddr_vm as *const libc::sockaddr)
845+
},
819846
mem::size_of_val(sa) as libc::socklen_t
820847
),
821848
}

0 commit comments

Comments
 (0)