Thread: ATL bug of CComPtr?

  1. #1
    Registered User
    Join Date
    May 2006
    Posts
    1,579

    ATL bug of CComPtr?

    Hello everyone,


    In the ATL Internals book, one form of constructor of CComQIPtr is implemented as this,

    Code:
    CComQIPtr (IUnknown* lp)
    {
        p = NULL; if (lp != NULL) lp -> QueryInterface (*piid, (void**)&p);
    }
    I think there is a bug when QueryInterface fails, and the original value of member variable p is overwritten.

    I found the in MSVC 2008, the implementation is,

    Code:
    atlcomcli.h
    
    	CComQIPtr(_In_opt_ IUnknown* lp) throw()
    	{
    		if (lp != NULL)
    			lp->QueryInterface(*piid, (void **)&p);
    	}
    Seems the bug is fixed? Is it a bug in old version of ATL or a bug in the book? :-)


    thanks in advance,
    George

  2. #2
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    >> Is it a bug in old version of ATL or a bug in the book?
    Probably.

    gg

  3. #3
    Registered User
    Join Date
    May 2006
    Posts
    1,579
    Thanks gg,


    Quote Originally Posted by Codeplug View Post
    >> Is it a bug in old version of ATL or a bug in the book?
    Probably.

    gg
    In my original post, you think the current implementation (MSVC 2008) of CComQIPtr is ok?


    regards,
    George

  4. #4
    Registered User Codeplug's Avatar
    Join Date
    Mar 2003
    Posts
    4,981
    Actually, I don't think it's a bug. It's probably an optimization since IUnknown::QueryInterafce() will set it to NULL on failure.

    gg

  5. #5
    Registered User
    Join Date
    May 2006
    Posts
    1,579
    Thanks gg,


    Is it possible that internal data member p holds some reference to other instance before we invoke this constructor?

    If yes, I think we should call Release on internal data member p, before construction again, in order to release the previous held reference, before gain the reference to the new instance.

    Quote Originally Posted by Codeplug View Post
    Actually, I don't think it's a bug. It's probably an optimization since IUnknown::QueryInterafce() will set it to NULL on failure.

    gg

    regards,
    George

  6. #6
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Quote Originally Posted by George2 View Post
    Is it possible that internal data member p holds some reference to other instance before we invoke this constructor?

    If yes, I think we should call Release on internal data member p, before construction again, in order to release the previous held reference, before gain the reference to the new instance.
    I think Codeplug is right. It was never a bug because the code in question is in the constructor. There is no previous value of the member variable. There is nothing to release as the contents are garbage until first set here. You must be thinking of the Attach method.
    Removing the first statement will just be an optimisation.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  7. #7
    Registered User
    Join Date
    May 2006
    Posts
    1,579
    Thanks iMalc,


    Question answered.

    Quote Originally Posted by iMalc View Post
    I think Codeplug is right. It was never a bug because the code in question is in the constructor. There is no previous value of the member variable. There is nothing to release as the contents are garbage until first set here. You must be thinking of the Attach method.
    Removing the first statement will just be an optimisation.

    regards,
    George

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Problem with atl memory cleaning
    By Mariam1989 in forum Windows Programming
    Replies: 1
    Last Post: 11-11-2008, 12:35 PM
  2. Good ATL Resource
    By DV64h in forum C++ Programming
    Replies: 0
    Last Post: 07-23-2006, 07:03 AM
  3. a browser control in a ATL DLL
    By lek in forum C++ Programming
    Replies: 2
    Last Post: 11-18-2005, 02:43 PM
  4. ATL exception handling
    By rzcodeman in forum Windows Programming
    Replies: 1
    Last Post: 06-10-2004, 06:19 PM
  5. MSVC and ATL problem
    By Unregistered in forum Windows Programming
    Replies: 1
    Last Post: 01-22-2002, 07:07 AM