From 35d9c5778c1d4802b7bd539b8b31812cadd40c04 Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Wed, 21 Aug 2024 17:01:14 +0200 Subject: [PATCH 1/6] Update to windows 0.58 --- Cargo.toml | 3 ++- src/connection.rs | 2 +- src/query_sink.rs | 2 +- src/result_enumerator.rs | 3 ++- src/safearray.rs | 1 + src/variant.rs | 30 +++++++++++++++++------------- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index db4191e..23d5f27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,8 @@ default = ["chrono"] test = [] [target.'cfg(target_os = "windows")'.dependencies] -windows = { version = "0.52", features = [ +windows-core = { version = "0.58" } +windows = { version = "0.58", features = [ "implement", "Win32_Foundation", "Win32_Security", diff --git a/src/connection.rs b/src/connection.rs index 1c6f2da..8d907c6 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -63,7 +63,7 @@ impl COMLibrary { /// `CoInitialize`s the COM library for use by the calling thread, but without setting the security context. /// pub fn without_security() -> WMIResult { - unsafe { CoInitializeEx(None, COINIT_MULTITHREADED)? } + unsafe { CoInitializeEx(None, COINIT_MULTITHREADED).ok()? } let instance = Self { _phantom: PhantomData, diff --git a/src/query_sink.rs b/src/query_sink.rs index 85706ca..ffce35a 100644 --- a/src/query_sink.rs +++ b/src/query_sink.rs @@ -145,7 +145,7 @@ pub struct QuerySink { /// receives asynchronously the result of the query, through Indicate calls. /// When finished,the SetStatus method is called. /// # -impl IWbemObjectSink_Impl for QuerySink { +impl IWbemObjectSink_Impl for QuerySink_Impl { fn Indicate( &self, lObjectCount: i32, diff --git a/src/result_enumerator.rs b/src/result_enumerator.rs index 94fd172..356e9cf 100644 --- a/src/result_enumerator.rs +++ b/src/result_enumerator.rs @@ -9,8 +9,9 @@ use serde::{ Serialize, }; use std::ptr; +use windows::core::VARIANT; use windows::Win32::System::Ole::SafeArrayDestroy; -use windows::Win32::System::Variant::{VariantClear, VARIANT}; +use windows::Win32::System::Variant::VariantClear; use windows::Win32::System::Wmi::{ IEnumWbemClassObject, IWbemClassObject, CIMTYPE_ENUMERATION, WBEM_FLAG_ALWAYS, WBEM_FLAG_NONSYSTEM_ONLY, WBEM_INFINITE, diff --git a/src/safearray.rs b/src/safearray.rs index b12851c..1d876e1 100644 --- a/src/safearray.rs +++ b/src/safearray.rs @@ -4,6 +4,7 @@ use crate::{ }; use std::{iter::Iterator, ptr::null_mut, slice}; use windows::core::BSTR; +// use windows_core::imp::SAFEARRAY; use windows::Win32::System::Com::SAFEARRAY; use windows::Win32::System::Ole::{ SafeArrayAccessData, SafeArrayGetLBound, SafeArrayGetUBound, SafeArrayUnaccessData, diff --git a/src/variant.rs b/src/variant.rs index 6fa7ef4..18b60bb 100644 --- a/src/variant.rs +++ b/src/variant.rs @@ -3,10 +3,11 @@ use crate::{ }; use serde::Serialize; use std::convert::TryFrom; -use windows::core::{ComInterface, IUnknown, BSTR}; -use windows::Win32::Foundation::{VARIANT_FALSE, VARIANT_TRUE}; +use windows::core::{IUnknown, BSTR, VARIANT}; +use windows::Win32::Foundation::{VARIANT_BOOL, VARIANT_FALSE, VARIANT_TRUE}; use windows::Win32::System::Variant::*; use windows::Win32::System::Wmi::{self, IWbemClassObject, CIMTYPE_ENUMERATION}; +use windows_core::Interface; #[derive(Debug, PartialEq, Serialize)] #[serde(untagged)] @@ -79,15 +80,19 @@ impl Variant { /// /// This function is unsafe as it is the caller's responsibility to ensure that the VARIANT is correctly initialized. pub fn from_variant(vt: &VARIANT) -> WMIResult { + let vt = vt.as_raw(); let variant_type = unsafe { vt.Anonymous.Anonymous.vt }; // variant_type has two 'forms': // 1. A simple type like `VT_BSTR` . // 2. An array of certain type like `VT_ARRAY | VT_BSTR`. - if variant_type.0 & VT_ARRAY.0 == VT_ARRAY.0 { - let array = unsafe { vt.Anonymous.Anonymous.Anonymous.parray }; + if variant_type & VT_ARRAY.0 == VT_ARRAY.0 { + let array = unsafe { + vt.Anonymous.Anonymous.Anonymous.parray + as *const windows::Win32::System::Com::SAFEARRAY + }; - let item_type = VARENUM(variant_type.0 & VT_TYPEMASK.0); + let item_type = VARENUM(variant_type & VT_TYPEMASK.0); return Ok(Variant::Array(unsafe { safe_array_to_vec(&*array, item_type)? @@ -97,10 +102,9 @@ impl Variant { // See https://msdn.microsoft.com/en-us/library/cc237865.aspx for more info. // Rust can infer the return type of `vt.*Val()` calls, // but it's easier to read when the type is named explicitly. - let variant_value = match variant_type { + let variant_value = match VARENUM(variant_type) { VT_BSTR => { - let bstr_ptr: &BSTR = unsafe { &vt.Anonymous.Anonymous.Anonymous.bstrVal }; - + let bstr_ptr = unsafe { BSTR::from_raw(vt.Anonymous.Anonymous.Anonymous.bstrVal) }; Variant::String(bstr_ptr.try_into()?) } VT_I1 => { @@ -136,10 +140,10 @@ impl Variant { VT_BOOL => { let value = unsafe { vt.Anonymous.Anonymous.Anonymous.boolVal }; - match value { + match VARIANT_BOOL(value) { VARIANT_FALSE => Variant::Bool(false), VARIANT_TRUE => Variant::Bool(true), - _ => return Err(WMIError::ConvertBoolError(value.0)), + _ => return Err(WMIError::ConvertBoolError(value)), } } VT_UI1 => { @@ -165,11 +169,11 @@ impl Variant { VT_EMPTY => Variant::Empty, VT_NULL => Variant::Null, VT_UNKNOWN => { - let unk = unsafe { &vt.Anonymous.Anonymous.Anonymous.punkVal }; - let ptr = unk.as_ref().ok_or(WMIError::NullPointerResult)?; + let unk = unsafe { &(vt.Anonymous.Anonymous.Anonymous.punkVal as *const IUnknown) }; + let ptr = unsafe { unk.as_ref() }.ok_or(WMIError::NullPointerResult)?; Variant::Unknown(IUnknownWrapper::new(ptr.clone())) } - _ => return Err(WMIError::ConvertError(variant_type.0)), + _ => return Err(WMIError::ConvertError(variant_type)), }; Ok(variant_value) From 06aecd8955342dfbefe5734d54821a383e82ec3a Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Wed, 21 Aug 2024 17:31:48 +0200 Subject: [PATCH 2/6] Compile tests --- src/query_sink.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query_sink.rs b/src/query_sink.rs index ffce35a..849238f 100644 --- a/src/query_sink.rs +++ b/src/query_sink.rs @@ -209,7 +209,7 @@ mod tests { use super::*; use crate::tests::fixtures::*; use futures::StreamExt; - use windows::core::{ComInterface, IUnknown, Interface}; + use windows::core::{IUnknown, Interface}; #[async_std::test] async fn async_it_should_send_result() { From 6271aa62fc32e2c276e35acd56454bfbc5da25ce Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Wed, 21 Aug 2024 17:34:01 +0200 Subject: [PATCH 3/6] Feedback --- src/safearray.rs | 1 - src/variant.rs | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/safearray.rs b/src/safearray.rs index 1d876e1..b12851c 100644 --- a/src/safearray.rs +++ b/src/safearray.rs @@ -4,7 +4,6 @@ use crate::{ }; use std::{iter::Iterator, ptr::null_mut, slice}; use windows::core::BSTR; -// use windows_core::imp::SAFEARRAY; use windows::Win32::System::Com::SAFEARRAY; use windows::Win32::System::Ole::{ SafeArrayAccessData, SafeArrayGetLBound, SafeArrayGetUBound, SafeArrayUnaccessData, diff --git a/src/variant.rs b/src/variant.rs index 18b60bb..779e005 100644 --- a/src/variant.rs +++ b/src/variant.rs @@ -3,11 +3,10 @@ use crate::{ }; use serde::Serialize; use std::convert::TryFrom; -use windows::core::{IUnknown, BSTR, VARIANT}; +use windows::core::{IUnknown, Interface, BSTR, VARIANT}; use windows::Win32::Foundation::{VARIANT_BOOL, VARIANT_FALSE, VARIANT_TRUE}; use windows::Win32::System::Variant::*; use windows::Win32::System::Wmi::{self, IWbemClassObject, CIMTYPE_ENUMERATION}; -use windows_core::Interface; #[derive(Debug, PartialEq, Serialize)] #[serde(untagged)] @@ -169,8 +168,7 @@ impl Variant { VT_EMPTY => Variant::Empty, VT_NULL => Variant::Null, VT_UNKNOWN => { - let unk = unsafe { &(vt.Anonymous.Anonymous.Anonymous.punkVal as *const IUnknown) }; - let ptr = unsafe { unk.as_ref() }.ok_or(WMIError::NullPointerResult)?; + let ptr = unsafe { IUnknown::from_raw(vt.Anonymous.Anonymous.Anonymous.punkVal) }; Variant::Unknown(IUnknownWrapper::new(ptr.clone())) } _ => return Err(WMIError::ConvertError(variant_type)), From 7443a5202d7c3b9acf7cd1dfc399afdfc29306b8 Mon Sep 17 00:00:00 2001 From: Ohad Ravid Date: Tue, 27 Aug 2024 22:09:13 +0300 Subject: [PATCH 4/6] Use `IUnknown::from_raw_borrowed` to avoid double free --- src/variant.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/variant.rs b/src/variant.rs index 779e005..996e25b 100644 --- a/src/variant.rs +++ b/src/variant.rs @@ -168,8 +168,9 @@ impl Variant { VT_EMPTY => Variant::Empty, VT_NULL => Variant::Null, VT_UNKNOWN => { - let ptr = unsafe { IUnknown::from_raw(vt.Anonymous.Anonymous.Anonymous.punkVal) }; - Variant::Unknown(IUnknownWrapper::new(ptr.clone())) + let ptr = unsafe { IUnknown::from_raw_borrowed(&vt.Anonymous.Anonymous.Anonymous.punkVal) }; + let ptr = ptr.cloned().ok_or(WMIError::NullPointerResult)?; + Variant::Unknown(IUnknownWrapper::new(ptr)) } _ => return Err(WMIError::ConvertError(variant_type)), }; From 6bfcddad735cfd9d3f3c1c128d3c9300c15b3def Mon Sep 17 00:00:00 2001 From: Ohad Ravid Date: Tue, 27 Aug 2024 22:09:53 +0300 Subject: [PATCH 5/6] To be safe, clone and leak the `BSTR` --- src/variant.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/variant.rs b/src/variant.rs index 996e25b..4a7c652 100644 --- a/src/variant.rs +++ b/src/variant.rs @@ -104,7 +104,10 @@ impl Variant { let variant_value = match VARENUM(variant_type) { VT_BSTR => { let bstr_ptr = unsafe { BSTR::from_raw(vt.Anonymous.Anonymous.Anonymous.bstrVal) }; - Variant::String(bstr_ptr.try_into()?) + let bstr_as_str = bstr_ptr.to_string(); + // We don't want to be the ones freeing the BSTR. + let _ = bstr_ptr.into_raw(); + Variant::String(bstr_as_str) } VT_I1 => { let num = unsafe { vt.Anonymous.Anonymous.Anonymous.cVal }; From c4ba53a39054b5a16278c61fec059b7233110eba Mon Sep 17 00:00:00 2001 From: jasper Date: Tue, 27 Aug 2024 22:12:05 +0200 Subject: [PATCH 6/6] fmt --- src/variant.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/variant.rs b/src/variant.rs index 4a7c652..2f9af39 100644 --- a/src/variant.rs +++ b/src/variant.rs @@ -171,7 +171,9 @@ impl Variant { VT_EMPTY => Variant::Empty, VT_NULL => Variant::Null, VT_UNKNOWN => { - let ptr = unsafe { IUnknown::from_raw_borrowed(&vt.Anonymous.Anonymous.Anonymous.punkVal) }; + let ptr = unsafe { + IUnknown::from_raw_borrowed(&vt.Anonymous.Anonymous.Anonymous.punkVal) + }; let ptr = ptr.cloned().ok_or(WMIError::NullPointerResult)?; Variant::Unknown(IUnknownWrapper::new(ptr)) }