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

Fix some clippy lints for library/std/src/sys/windows #118154

Merged
merged 17 commits into from
Nov 23, 2023
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())
Copy link
Member

Choose a reason for hiding this comment

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

Can we uplift this clippy lint? 🙂

(Not in this PR, obviously)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's needless_borrows_for_generic_args. There's also the related needless_borrow which found a lot too!

}

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) }
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this is PSTR, which is defined as *mut u8, so the change is fine, but I wonder if it's "really" *mut u8 vs *mut c_char, since the latter might want to be signed char, and where this cast would actually do something.

Since it's not using c_char today, though, it's probably fine, since casts can be added back if it's ever relevant.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true. Though we're a bit at the mercy of the win32metadata project here (which is what our bindings are ultimately generated from) which might not necessarily follow what we think a c_char is (as it has no particular need to follow C).

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) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I confirmed that using matches! is exactly equivalent to using a manual if (in terms of codegen) and I do think it's clearer.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, so long as there's only a single range in the matches! then it's just as good.

(If there's multiple then things get complicated, see #103024 (comment) if you're curious.)

// 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
Loading