|
|
Comments and Discussions
|
|
 |

|
I was seeing such an article about this CComPtr for the first time and it helped me a lot.
This portion explains many of the minute details of this pointer. I am grateful to the article and author.
|
|
|
|

|
Pointing goldsam was wrong doesn't prove you right. In fact, your wrapper is flawed in many ways. To state what a lot of your readers have already noticed :
- In all you accessors, you return your inner interface pointer. It ruins the whole encapsulation purpose intended by any smart-pointer/reference/handle. You should return *this; as a reference to self.
- Mixing run-time and compile-time use of the IID doesn't make sense. The IID should be statically available within your class. There is no need for a CComPtr(IUnknown* pIUnknown, IID iid) copy constructor. By the way, your implementation is not even consistent since you don't supply an operator= (IUnknown* pIUnknown, IID iid).
- operator* is irrevelant. COM interfaces are abstract thus not supposed to be dereferenced to their underlying struct. operator-> is sufficient.
- Implicit cast to the inner interface pointer is questionnable. The code you write, based on that wrapper, doesn't need such convertion. When giving away the actual interface to other APIs, a simple GetRawInterface() accessor is generally preferred.
- IsEqualObject should be redesigned to a symetrical operator== not only for cosmetics but because you MUST query for the address of BOTH the IUnknown. Another thing : you seem to prefer to call QueryInterface and Release on different interface pointers. This only obfuscate your code. Regarding IUnknown that ALL interfaces must implement, a COM object MUST behave the exact same way whatever interface the client use to access that particular object.
- Over ASSERTing every statement and setting m_Ptr to NULL in every way possible doesn't make your code more robust, it only show quite clearly you're not confident with how you're doing things.
- Talking about robustness, overloading operator& is awful : no well designed wrapper exposes its implementation details.
- You might also consider to make your class follow the RAII idiom so it can't be in a inconsistent state.
- And it goes on ...
To summer it up, while the use of smart-pointers is meant to be simple and natural, these little beasts are difficult to implement correctly and need very special care. See ?
It would have been a simple matter of respect to FIX your code when kind people took time to review it and pointed mistakes. Jumping on the first attempt of flamming just adds up to the pollution, your code included ... sorry. Writing articles here isn't about teaching, it is about sharing experiences so everybody can learn, you're included, not the other way around.
|
|
|
|

|
Check out the Comet[^] COM library - very cool stuff and they don't rely on ATL or VC's COM compiler extensions.
¡El diablo está en mis pantalones! ¡Mire, mire!
Real Mentats use only 100% pure, unfooled around with Sapho Juice(tm)!
SELECT * FROM User WHERE Clue > 0
0 rows returned
Save an Orange - Use the VCF!
|
|
|
|

|
Hi Jim,
Thank you for the recommendation. Comet looks cool. I will spend time in studying it in next few days. However I notice that the lastest version of Coment is release on 2004-9-26, one and half years ago. What happened to the owner? Are they too busy to continue the work? Or they gave up?
Skeeter
MCP, MCP+I, MCSE, MCSD & MCDBA
|
|
|
|

|
Not really sure. I've started to use it for some stuff in my projects, and on the forums, there's a link to a newer version, but I don't why the development seems to have stopped. But even in it's current state it's quite useful.
¡El diablo está en mis pantalones! ¡Mire, mire!
Real Mentats use only 100% pure, unfooled around with Sapho Juice(tm)!
SELECT * FROM User WHERE Clue > 0
0 rows returned
Save an Orange - Use the VCF!
|
|
|
|

|
This class is horribly flawed in logic as well as inefficient.
Consider the IsEqualObject(IUnknown* pOther) method:
First, there is no need to check if both are NULL.. they either equal each other or they don't. It doesn't make sense to compare the interfaces after a call to QueryInterface with IID = IID_IUnknown. m_Ptr is the interface already being held, so IT MUST BE AN IUnknown. The same goes with pOther... you already know all COM interfaces have IUnknown as a base... thats not what QueryInterface was designed to discover. YOU COULDN'T CALL QueryInterface IF THEY DIDN'T SUPPORT IT!! In addition to that total brain dead logic, you are leaking memory!!! you are only calling Release() on the interfaces if both point to an object, what happens if only one of the QueryInterface calls is succesful?
You also seem to have some obsession of setting m_Ptr to 0 in the most pointless places. Here is one of many tasty examples of your zeroing fetish:
CComPtr(const CComPtr& RefComPtr)
{
m_Ptr = NULL; <----------- WHY?!
m_Ptr = (INTERFACE*)RefComPtr; <----- this gonna be zero... or have a value
if (m_Ptr)
{
m_Ptr->AddRef();
}
}
In the future, you should really take the time to review you're code before you post it to a site like this. The inefficiencies are minor, buy the memory leak issue is hard to miss. Just slow down a bit.
-- modified at 13:46 Friday 28th October, 2005
|
|
|
|

|
Though I very appreciate your inputs, I think your attitude somehow has some problems. First fo all, your suggestion about IsEqualObject(IUnknown* pOther) method tells me you don't understand COM at all. A COM pointer doesn't necessarily points to IUnknown interface. It may points to other interfaces in the same COM object. That is why QueryInterface() must be called even if m_Ptr is there. Secondly my class is clearly a COM Smart Wrapper class, the pointer passed in MUST be a COM pointer. This is the requirement to the developer using my code. Because of this, it is safe for my code to call QueryInterface() on lPtr to get the IUnknown interface. In this situation, I think you are the one who is in total brain dead logic. Last but not least, why I set m_Ptr to 0 in most of the place is because in Visual C++ Compiler, the default value given to a pointer is 0xcccccccc instead of 0. After querying interface, I must check whether it is successful. I don't see any problem to set pointer to 0 and then validate it after QueryInterface() returns. This is not about efficiency. It is about code robutness. If you don't do that in your code, you definitely a sucks developer.
Though you are so unfriendly, I still value your inputs and look forward to more suggestions from you.
Skeeter
Skeeter
MCP, MCP+I, MCSE, MCSD & MCDBA
|
|
|
|

|
How can I activate a COM object in a remote machine? I allways get the message "Access is denied".
Júlio
|
|
|
|

|
About a month ago I reported bugs to the author so he could fix them. They have gone unfixed so I wanted to warn people.
1. All the operator = methods will leak a reference. When fixing this bug, be very careful that you don't make the mistake of releasing the previous interface then assigning the new. The can cause a bug if the new interface is the same as the current interface.
2. Aside from a minor optimization, the IsEqualObject method can leak a reference if one of the objects fails to return an interface for IUnknown. It is very true that this should never happen, but since the author is already testing for this case, he should properly handle it.
Tim Smith
I know what you're thinking punk, you're thinking did he spell check this document? Well, to tell you the truth I kinda forgot myself in all this excitement. But being this here's CodeProject, the most powerful forums in the world and would blow your head clean off, you've got to ask yourself one question, Do I feel lucky? Well do ya punk?
|
|
|
|

|
Tim Smith - BugSlayer
Go Tim Go....
Nish
My miniputt high is now 29
I do not think I can improve on that
My temperament won't hold
www.busterboy.org
|
|
|
|

|
what about class _com_ptr_t?
already exists and surely more reliable than yours!
|
|
|
|

|
Read the article
Tim Smith
I know what you're thinking punk, you're thinking did he spell check this document? Well, to tell you the truth I kinda forgot myself in all this excitement. But being this here's CodeProject, the most powerful forums in the world and would blow your head clean off, you've got to ask yourself one question, Do I feel lucky? Well do ya punk?
|
|
|
|

|
You wrote:
>But it uses some ATL related funtion so that it can be
>used only in ATL environment
Which functions ?
There are only 2 small and simple functions.
And you use COM on Linux & Mac ??
|
|
|
|

|
You can use COM on non Windows OS. I did it by implementing myself the INPROC support of the COM library, and since then we run our server both on NT and Solaris.
The source is free - from
http://www.geocities.com/ResearchTriangle/Facility/1124/software/software.html
enjoy.
Noam Cohen,
Vsoft.
|
|
|
|

|
What's going on ? Have you just discovered CComPtr ? It's been around for years and has been extremely well documented. Your article doesn't really explain a thing.
Very disappointing
|
|
|
|

|
The implementation described here is not the one described in MSDN - the author has created a new implementation for the reasons described in the article.
--
Andrew.
|
|
|
|

|
Every innovation is based on previous one. Nobody can create a new idea from empty. This com smart pointer is used in our driver programming architecture.
When talking about driver programming, maybe you will think it is too simple to use COM. Actually our driver is very complicated. It has its own hardware abstract layer, its own OS service layer, its own plug in layer. Every module is based on C++/COM. That is, we implemented our own IUnknown, INondelegationQuryInterface, etc. Let me say, we do all the Windows COM can do. Our driver can run under not only Windows platform but also MAC and LINUX. We have core code and OS specific code, which can be replaced smoothly. As a core developer of such driver project, I'd like to share my experience with other programmers. If you think this is useful to you, I feel very happy. If not, I have to say sorry and I hope you can do it better
and share your experience with me. Thank you.
Skeeter
------------------------------
MCP, MCP+I, MCSE, MCSD & MCDBA
|
|
|
|

|
Personally, I find this article very useful. The implementation is definitely different from the one found in MSDN. It is much easier to comprehend CComPtr by reading this article rather than MSDN, especially for "new-COMers". My vote is excellent.
I'd also be very happy if the author could spare some time, and share his experience about driver programming with COM, particularly WDM.
Thank you.
Aram Sargsyan
|
|
|
|
 |
|
|
General News Suggestion Question Bug Answer Joke Rant Admin
|
A wrapper class to any COM interface pointer.
| Type | Article |
| Licence | |
| First Posted | 14 Oct 2001 |
| Views | 144,934 |
| Bookmarked | 36 times |
|
|