Skip to content

Commit

Permalink
Auto merge of #118154 - ChrisDenton:win-clippy, r=scottmcm
Browse files Browse the repository at this point in the history
Fix some clippy lints for library/std/src/sys/windows

These issues were shown by running `x clippy` on `library/std` and filtering for `windows/` paths. I think running clippy on the full std would be great but I wanted to start smaller and with something that's hopefully easier to review. It'd be good to eventually run clippy in CI but that's a bigger conversation.

I've created separate commits for each clippy lint fixed (with the commit title set to the lint name) and reviewed the changes myself. Most of the fixes here are trivial.

r? libs
  • Loading branch information
bors committed Nov 23, 2023
2 parents 1e9dda7 + b9fe367 commit c387f01
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 45 deletions.
2 changes: 1 addition & 1 deletion library/std/src/os/windows/io/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ impl AsHandle for fs::File {
impl From<fs::File> for OwnedHandle {
#[inline]
fn from(file: fs::File) -> OwnedHandle {
file.into_inner().into_inner().into_inner().into()
file.into_inner().into_inner().into_inner()
}
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/os/windows/io/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl BorrowedSocket<'_> {
info.iAddressFamily,
info.iSocketType,
info.iProtocol,
&mut info,
&info,
0,
sys::c::WSA_FLAG_OVERLAPPED | sys::c::WSA_FLAG_NO_HANDLE_INHERIT,
)
Expand All @@ -147,7 +147,7 @@ impl BorrowedSocket<'_> {
info.iAddressFamily,
info.iSocketType,
info.iProtocol,
&mut info,
&info,
0,
sys::c::WSA_FLAG_OVERLAPPED,
)
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/windows/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub fn set_file_information_by_handle<T: SetFileInformation>(
size: u32,
) -> Result<(), WinError> {
let result = c::SetFileInformationByHandle(handle, class, info, size);
(result != 0).then_some(()).ok_or_else(|| get_last_error())
(result != 0).then_some(()).ok_or_else(get_last_error)
}
// SAFETY: The `SetFileInformation` trait ensures that this is safe.
unsafe { set_info(handle, T::CLASS, info.as_ptr(), info.size()) }
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![allow(nonstandard_style)]
#![cfg_attr(test, allow(dead_code))]
#![unstable(issue = "none", feature = "windows_c")]
#![allow(clippy::style)]

use crate::ffi::CStr;
use crate::mem;
Expand Down Expand Up @@ -81,7 +82,7 @@ pub fn nt_success(status: NTSTATUS) -> bool {

impl UNICODE_STRING {
pub fn from_ref(slice: &[u16]) -> Self {
let len = slice.len() * mem::size_of::<u16>();
let len = mem::size_of_val(slice);
Self { Length: len as _, MaximumLength: len as _, Buffer: slice.as_ptr() as _ }
}
}
Expand Down
14 changes: 7 additions & 7 deletions library/std/src/sys/windows/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl DirEntry {
}

pub fn path(&self) -> PathBuf {
self.root.join(&self.file_name())
self.root.join(self.file_name())
}

pub fn file_name(&self) -> OsString {
Expand Down Expand Up @@ -548,7 +548,7 @@ impl File {
let user = super::args::from_wide_to_user_path(
subst.iter().copied().chain([0]).collect(),
)?;
Ok(PathBuf::from(OsString::from_wide(&user.strip_suffix(&[0]).unwrap_or(&user))))
Ok(PathBuf::from(OsString::from_wide(user.strip_suffix(&[0]).unwrap_or(&user))))
} else {
Ok(PathBuf::from(OsString::from_wide(subst)))
}
Expand Down Expand Up @@ -786,7 +786,7 @@ fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result<
// tricked into following a symlink. However, it may not be available in
// earlier versions of Windows.
static ATTRIBUTES: AtomicU32 = AtomicU32::new(c::OBJ_DONT_REPARSE);
let mut object = c::OBJECT_ATTRIBUTES {
let object = c::OBJECT_ATTRIBUTES {
ObjectName: &mut name_str,
RootDirectory: parent.as_raw_handle(),
Attributes: ATTRIBUTES.load(Ordering::Relaxed),
Expand All @@ -795,7 +795,7 @@ fn open_link_no_reparse(parent: &File, name: &[u16], access: u32) -> io::Result<
let status = c::NtCreateFile(
&mut handle,
access,
&mut object,
&object,
&mut io_status,
crate::ptr::null_mut(),
0,
Expand Down Expand Up @@ -874,7 +874,7 @@ impl fmt::Debug for File {
// FIXME(#24570): add more info here (e.g., mode)
let mut b = f.debug_struct("File");
b.field("handle", &self.handle.as_raw_handle());
if let Ok(path) = get_path(&self) {
if let Ok(path) = get_path(self) {
b.field("path", &path);
}
b.finish()
Expand Down Expand Up @@ -1193,7 +1193,7 @@ pub fn readlink(path: &Path) -> io::Result<PathBuf> {
let mut opts = OpenOptions::new();
opts.access_mode(0);
opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT | c::FILE_FLAG_BACKUP_SEMANTICS);
let file = File::open(&path, &opts)?;
let file = File::open(path, &opts)?;
file.readlink()
}

Expand Down Expand Up @@ -1407,7 +1407,7 @@ pub fn symlink_junction<P: AsRef<Path>, Q: AsRef<Path>>(
#[allow(dead_code)]
fn symlink_junction_inner(original: &Path, junction: &Path) -> io::Result<()> {
let d = DirBuilder::new();
d.mkdir(&junction)?;
d.mkdir(junction)?;

let mut opts = OpenOptions::new();
opts.write(true);
Expand Down
10 changes: 5 additions & 5 deletions library/std/src/sys/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl Handle {
let res = unsafe { self.synchronous_read(buf.as_mut_ptr().cast(), buf.len(), None) };

match res {
Ok(read) => Ok(read as usize),
Ok(read) => Ok(read),

// The special treatment of BrokenPipe is to deal with Windows
// pipe semantics, which yields this error when *reading* from
Expand All @@ -107,7 +107,7 @@ impl Handle {
unsafe { self.synchronous_read(buf.as_mut_ptr().cast(), buf.len(), Some(offset)) };

match res {
Ok(read) => Ok(read as usize),
Ok(read) => Ok(read),
Err(ref e) if e.raw_os_error() == Some(c::ERROR_HANDLE_EOF as i32) => Ok(0),
Err(e) => Err(e),
}
Expand All @@ -121,7 +121,7 @@ impl Handle {
Ok(read) => {
// Safety: `read` bytes were written to the initialized portion of the buffer
unsafe {
cursor.advance(read as usize);
cursor.advance(read);
}
Ok(())
}
Expand Down Expand Up @@ -189,7 +189,7 @@ impl Handle {
}

pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
self.synchronous_write(&buf, None)
self.synchronous_write(buf, None)
}

pub fn write_vectored(&self, bufs: &[IoSlice<'_>]) -> io::Result<usize> {
Expand All @@ -202,7 +202,7 @@ impl Handle {
}

pub fn write_at(&self, buf: &[u8], offset: u64) -> io::Result<usize> {
self.synchronous_write(&buf, Some(offset))
self.synchronous_write(buf, Some(offset))
}

pub fn try_clone(&self) -> io::Result<Self> {
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/windows/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl<'a> IoSlice<'a> {

#[inline]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(self.vec.buf as *mut u8, self.vec.len as usize) }
unsafe { slice::from_raw_parts(self.vec.buf, self.vec.len as usize) }
}
}

Expand Down Expand Up @@ -70,12 +70,12 @@ impl<'a> IoSliceMut<'a> {

#[inline]
pub fn as_slice(&self) -> &[u8] {
unsafe { slice::from_raw_parts(self.vec.buf as *mut u8, self.vec.len as usize) }
unsafe { slice::from_raw_parts(self.vec.buf, self.vec.len as usize) }
}

#[inline]
pub fn as_mut_slice(&mut self) -> &mut [u8] {
unsafe { slice::from_raw_parts_mut(self.vec.buf as *mut u8, self.vec.len as usize) }
unsafe { slice::from_raw_parts_mut(self.vec.buf, self.vec.len as usize) }
}
}

Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub unsafe fn init(_argc: isize, _argv: *const *const u8, _sigpipe: u8) {

// Normally, `thread::spawn` will call `Thread::set_name` but since this thread already
// exists, we have to call it ourselves.
thread::Thread::set_name(&CStr::from_bytes_with_nul_unchecked(b"main\0"));
thread::Thread::set_name(CStr::from_bytes_with_nul_unchecked(b"main\0"));
}

// SAFETY: must be called only once during runtime cleanup.
Expand Down Expand Up @@ -150,7 +150,7 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind {

pub fn unrolled_find_u16s(needle: u16, haystack: &[u16]) -> Option<usize> {
let ptr = haystack.as_ptr();
let mut start = &haystack[..];
let mut start = haystack;

// For performance reasons unfold the loop eight times.
while start.len() >= 8 {
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/windows/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl Socket {

let mut timeout = c::timeval {
tv_sec: cmp::min(timeout.as_secs(), c_long::MAX as u64) as c_long,
tv_usec: (timeout.subsec_nanos() / 1000) as c_long,
tv_usec: timeout.subsec_micros() as c_long,
};

if timeout.tv_sec == 0 && timeout.tv_usec == 0 {
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/windows/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
let k = to_u16s(k).ok()?;
super::fill_utf16_buf(
|buf, sz| unsafe { c::GetEnvironmentVariableW(k.as_ptr(), buf, sz) },
|buf| OsStringExt::from_wide(buf),
OsStringExt::from_wide,
)
.ok()
}
Expand Down Expand Up @@ -356,13 +356,13 @@ pub fn home_dir() -> Option<PathBuf> {
crate::env::var_os("HOME")
.or_else(|| crate::env::var_os("USERPROFILE"))
.map(PathBuf::from)
.or_else(|| home_dir_crt())
.or_else(home_dir_crt)
}

pub fn exit(code: i32) -> ! {
unsafe { c::ExitProcess(code as c::UINT) }
}

pub fn getpid() -> u32 {
unsafe { c::GetCurrentProcessId() as u32 }
unsafe { c::GetCurrentProcessId() }
}
12 changes: 5 additions & 7 deletions library/std/src/sys/windows/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<'a> PrefixParserSlice<'a, '_> {
fn strip_prefix(&self, prefix: &str) -> Option<Self> {
self.prefix[self.index..]
.starts_with(prefix.as_bytes())
.then(|| Self { index: self.index + prefix.len(), ..*self })
.then_some(Self { index: self.index + prefix.len(), ..*self })
}

fn prefix_bytes(&self) -> &'a [u8] {
Expand Down Expand Up @@ -147,12 +147,10 @@ pub fn parse_prefix(path: &OsStr) -> Option<Prefix<'_>> {
None
}
}
} else if let Some(drive) = parse_drive(path) {
// C:
Some(Disk(drive))
} else {
// no prefix
None
// If it has a drive like `C:` then it's a disk.
// Otherwise there is no prefix.
parse_drive(path).map(Disk)
}
}

Expand Down Expand Up @@ -252,7 +250,7 @@ pub(crate) fn get_long_path(mut path: Vec<u16>, prefer_verbatim: bool) -> io::Re
// \\?\UNC\
const UNC_PREFIX: &[u16] = &[SEP, SEP, QUERY, SEP, U, N, C, SEP];

if path.starts_with(VERBATIM_PREFIX) || path.starts_with(NT_PREFIX) || path == &[0] {
if path.starts_with(VERBATIM_PREFIX) || path.starts_with(NT_PREFIX) || path == [0] {
// Early return for paths that are already verbatim or empty.
return Ok(path);
} else if path.len() < LEGACY_MAX_PATH {
Expand Down
11 changes: 5 additions & 6 deletions library/std/src/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ impl Command {
}

pub fn get_current_dir(&self) -> Option<&Path> {
self.cwd.as_ref().map(|cwd| Path::new(cwd))
self.cwd.as_ref().map(Path::new)
}

pub unsafe fn raw_attribute<T: Copy + Send + Sync + 'static>(
Expand Down Expand Up @@ -463,7 +463,7 @@ fn resolve_exe<'a>(

// Search the directories given by `search_paths`.
let result = search_paths(parent_paths, child_paths, |mut path| {
path.push(&exe_path);
path.push(exe_path);
if !has_extension {
path.set_extension(EXE_EXTENSION);
}
Expand Down Expand Up @@ -657,7 +657,7 @@ impl Process {
}

pub fn id(&self) -> u32 {
unsafe { c::GetProcessId(self.handle.as_raw_handle()) as u32 }
unsafe { c::GetProcessId(self.handle.as_raw_handle()) }
}

pub fn main_thread_handle(&self) -> BorrowedHandle<'_> {
Expand Down Expand Up @@ -917,9 +917,8 @@ fn make_proc_thread_attribute_list(
)
};

let mut proc_thread_attribute_list = ProcThreadAttributeList(
vec![MaybeUninit::uninit(); required_size as usize].into_boxed_slice(),
);
let mut proc_thread_attribute_list =
ProcThreadAttributeList(vec![MaybeUninit::uninit(); required_size].into_boxed_slice());

// Once we've allocated the necessary memory, it's safe to invoke
// `InitializeProcThreadAttributeList` to properly initialize the list.
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/windows/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ fn write_valid_utf8_to_console(handle: c::HANDLE, utf8: &str) -> io::Result<usiz
MaybeUninit::slice_assume_init_ref(&utf16[..result as usize])
};

let mut written = write_u16s(handle, &utf16)?;
let mut written = write_u16s(handle, utf16)?;

// Figure out how many bytes of as UTF-8 were written away as UTF-16.
if written == utf16.len() {
Expand All @@ -207,7 +207,7 @@ fn write_valid_utf8_to_console(handle: c::HANDLE, utf8: &str) -> io::Result<usiz
// write the missing surrogate out now.
// Buffering it would mean we have to lie about the number of bytes written.
let first_code_unit_remaining = utf16[written];
if first_code_unit_remaining >= 0xDCEE && first_code_unit_remaining <= 0xDFFF {
if matches!(first_code_unit_remaining, 0xDCEE..=0xDFFF) {
// low surrogate
// We just hope this works, and give up otherwise
let _ = write_u16s(handle, &utf16[written..written + 1]);
Expand Down Expand Up @@ -266,7 +266,7 @@ impl io::Read for Stdin {
let mut bytes_copied = self.incomplete_utf8.read(buf);

if bytes_copied == buf.len() {
return Ok(bytes_copied);
Ok(bytes_copied)
} else if buf.len() - bytes_copied < 4 {
// Not enough space to get a UTF-8 byte. We will use the incomplete UTF8.
let mut utf16_buf = [MaybeUninit::new(0); 1];
Expand Down Expand Up @@ -332,7 +332,7 @@ fn read_u16s_fixup_surrogates(
// and it is not 0, so we know that `buf[amount - 1]` have been
// initialized.
let last_char = unsafe { buf[amount - 1].assume_init() };
if last_char >= 0xD800 && last_char <= 0xDBFF {
if matches!(last_char, 0xD800..=0xDBFF) {
// high surrogate
*surrogate = last_char;
amount -= 1;
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/windows/time.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::cmp::Ordering;
use crate::fmt;
use crate::mem;
use crate::ptr::{null, null_mut};
use crate::ptr::null;
use crate::sys::c;
use crate::sys_common::IntoInner;
use crate::time::Duration;
Expand Down Expand Up @@ -240,7 +240,7 @@ impl WaitableTimer {
c::TIMER_ALL_ACCESS,
)
};
if handle != null_mut() { Ok(Self { handle }) } else { Err(()) }
if !handle.is_null() { Ok(Self { handle }) } else { Err(()) }
}
pub fn set(&self, duration: Duration) -> Result<(), ()> {
// Convert the Duration to a format similar to FILETIME.
Expand Down

0 comments on commit c387f01

Please sign in to comment.