Merge pull request #28 from microsoft/com-ptr-initialization

Mark ComPtr::new as unsafe

examples/basic/client/src/main.rs

@@ -201,9 +201,7 @@ fn get_class_object(iid: &IID) -> Result<ComPtr<IClassFactory>, HRESULT> {
         return Err(hr);
     }
 
-    Ok(ComPtr::new(
+    unsafe { Ok(ComPtr::new(class_factory)) }
-        std::ptr::NonNull::new(class_factory as *mut c_void).unwrap(),
-    ))
 }
 
 // TODO: accept server options
@@ -222,9 +220,7 @@ fn create_instance<T: ComInterface + ?Sized>(clsid: &IID) -> Result<ComPtr<T>, H
         return Err(hr);
     }
 
-    Ok(ComPtr::new(
+    unsafe { Ok(ComPtr::new(instance)) }
-        std::ptr::NonNull::new(instance as *mut c_void).unwrap(),
-    ))
 }
 
 fn uninitialize() {

examples/basic/server/src/local_file_manager.rs

@@ -87,7 +87,7 @@ unsafe extern "stdcall" fn ilocalfilemanager_query_interface(
     ppv: *mut *mut c_void,
 ) -> HRESULT {
     let lfm = this as *mut LocalFileManager;
-    let mut iunk_to_use : ComPtr<IUnknown> = ComPtr::new(NonNull::new((*lfm).iunk_to_use as *mut c_void).unwrap());
+    let mut iunk_to_use : ComPtr<IUnknown> = ComPtr::new((*lfm).iunk_to_use as *mut c_void);
     let hr = iunk_to_use.query_interface(riid, ppv);
     forget(iunk_to_use);
     
@@ -96,7 +96,7 @@ unsafe extern "stdcall" fn ilocalfilemanager_query_interface(
 
 unsafe extern "stdcall" fn ilocalfilemanager_add_ref(this: *mut IUnknownVPtr) -> u32 {
     let lfm = this as *mut LocalFileManager;
-    let mut iunk_to_use : ComPtr<IUnknown> = ComPtr::new(NonNull::new((*lfm).iunk_to_use as *mut c_void).unwrap());
+    let mut iunk_to_use : ComPtr<IUnknown> = ComPtr::new((*lfm).iunk_to_use as *mut c_void);
     let hr = iunk_to_use.add_ref();
     forget(iunk_to_use);
     
@@ -105,7 +105,7 @@ unsafe extern "stdcall" fn ilocalfilemanager_add_ref(this: *mut IUnknownVPtr) ->
 
 unsafe extern "stdcall" fn ilocalfilemanager_release(this: *mut IUnknownVPtr) -> u32 {
     let lfm = this as *mut LocalFileManager;
-    let mut iunk_to_use : ComPtr<IUnknown> = ComPtr::new(NonNull::new((*lfm).iunk_to_use as *mut c_void).unwrap());
+    let mut iunk_to_use : ComPtr<IUnknown> = ComPtr::new((*lfm).iunk_to_use as *mut c_void);
     let hr = iunk_to_use.release();
     forget(iunk_to_use);
     

examples/basic/server/src/local_file_manager_class.rs

@@ -52,8 +52,7 @@ impl IClassFactory for LocalFileManagerClass {
 
             // Here, we create a ComPtr since it is the only way to call IUnknown methods. We also add_ref here, as 
             // ComPtr will call release at the end of this scope.
-            let mut non_delegating_unk : ComPtr<IUnknown> = ComPtr::new(NonNull::new(&lfm.non_delegating_unk as *const _ as *mut c_void).unwrap());
+            let mut non_delegating_unk : ComPtr<IUnknown> = ComPtr::new(&lfm.non_delegating_unk as *const _ as *mut c_void);
-            // non_delegating_unk.add_ref();
 
             // As an aggregable object, we have to add_ref through the
             // non-delegating IUnknown on creation. Otherwise, we might

examples/basic/server/src/windows_file_manager.rs

@@ -28,7 +28,7 @@ pub struct WindowsFileManager {
 impl Drop for WindowsFileManager {
     fn drop(&mut self) {
         unsafe {
-            let mut lfm_iunknown : ComPtr<IUnknown> = ComPtr::new(NonNull::new(self.lfm_iunknown as *mut c_void).unwrap());
+            let mut lfm_iunknown : ComPtr<IUnknown> = ComPtr::new(self.lfm_iunknown as *mut c_void);
             lfm_iunknown.release();
             Box::from_raw(self.inner_one as *mut IFileManagerVTable);
 
@@ -53,7 +53,7 @@ impl IUnknown for WindowsFileManager {
                 *ppv = self as *const _ as *mut c_void;
             } else if IsEqualGUID(riid, &IID_ILOCAL_FILE_MANAGER) {
 
-                let mut lfm_iunknown : ComPtr<IUnknown> = ComPtr::new(NonNull::new(self.lfm_iunknown as *mut c_void).unwrap());
+                let mut lfm_iunknown : ComPtr<IUnknown> = ComPtr::new(self.lfm_iunknown as *mut c_void);
                 let hr = lfm_iunknown.query_interface(riid, ppv);
                 if failed(hr) {
                     return E_NOINTERFACE;

examples/safe/client/src/main.rs

@@ -113,9 +113,7 @@ fn get_class_object(iid: &IID) -> Result<ComPtr<IClassFactory>, HRESULT> {
         return Err(hr);
     }
 
-    Ok(ComPtr::new(
+    unsafe { Ok(ComPtr::new(class_factory)) }
-        std::ptr::NonNull::new(class_factory as *mut c_void).unwrap(),
-    ))
 }
 
 // TODO: accept server options
@@ -134,9 +132,7 @@ fn create_instance<T: ComInterface + ?Sized>(clsid: &IID) -> Result<ComPtr<T>, H
         return Err(hr);
     }
 
-    Ok(ComPtr::new(
+    unsafe { Ok(ComPtr::new(instance)) }
-        std::ptr::NonNull::new(instance as *mut c_void).unwrap(),
-    ))
 }
 
 fn uninitialize() {

src/comptr.rs

@@ -13,9 +13,18 @@ pub struct ComPtr<T: ComInterface + ?Sized> {
 }
 
 impl<T: ComInterface + ?Sized> ComPtr<T> {
-    pub fn new(ptr: NonNull<c_void>) -> Self {
+    /// Creates a new ComPtr that comforms to the interface T
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must point to a valid VTable for the Interface T
+    ///
+    /// # Panics
+    ///
+    /// Panics if `ptr` is null
+    pub unsafe fn new(ptr: *mut c_void) -> ComPtr<T> {
         ComPtr {
-            ptr,
+            ptr: NonNull::new(ptr).expect("ptr was null"),
             phantom: PhantomData,
         }
     }
@@ -44,7 +53,7 @@ impl<T: ComInterface + ?Sized> ComPtr<T> {
             assert!(hr == E_NOINTERFACE);
             return None;
         }
-        Some(ComPtr::new(std::ptr::NonNull::new(ppv as *mut c_void)?))
+        unsafe { Some(ComPtr::new(ppv)) }
     }
 }
 

src/iclassfactory.rs

@@ -25,7 +25,7 @@ impl ComPtr<IClassFactory> {
             // TODO: decide what failures are possible
             return None;
         }
-        Some(ComPtr::new(std::ptr::NonNull::new(ppv as *mut c_void)?))
+        unsafe { Some(ComPtr::new(ppv)) }
     }
 }