4

Here's a code sample creating a COM object:

CComPtr<IBaseFilter> pFilter;
auto hr = CoCreateInstance(CLSID_DMOWrapperFilter, NULL,
    CLSCTX_INPROC_SERVER, IID_IBaseFilter, reinterpret_cast<void**>(&pFilter));

I've seen somewhere that checking if CoCreateInstance() succeeded should look like this:

if (SUCCEEDED(hr) && pFilter != nullptr)
{
  // code goes here
}

What if I would check only hr? Wouldn't it be enough? Should I also check that filter != nullptr?

//would this be enough?
if (SUCCEEDED(hr))
{
  // code goes here
}

This question also concerns other COM methods like QueryInterface().

sharptooth
  • 167,383
  • 100
  • 513
  • 979
Dalamber
  • 1,009
  • 1
  • 12
  • 32

2 Answers2

6

Having S_OK result from CoCreateInstance you are guaranteed to get a non-NULL interface pointer, so you don't need to check it additionally. To make it more reliable and be able to detect issues early, you might want to use ATLASSERT there to compare against NULL. This does not produce code in release builds, but generates an early warning in debug if anything goes wrong (esp. you edit or copy paste code later and change the logic of obtaining the pointer.

CComPtr<IBaseFilter> pFilter;
HRESULT hr = CoCreateInstance(CLSID_DMOWrapperFilter, NULL, CLSCTX_INPROC_SERVER,
  IID_IBaseFilter, reinterpret_cast<VOID**>(&pFilter));
if(SUCCEEDED(hr))
{
  ATLASSERT(pFilter); // hr is of the interest because might explain failure
                      // pFilter is expected to be non-NULL in case of S_OK
  const CComQIPtr<IDMOWrapperFilter> pDmoWrapperFilter = pFilter;
  if(pDmoWrapperFilter)
  {
    // We're not really interested in QueryInterface's HRESULT since it's
    // either S_OK or E_NOINTERFACE, hr will typically explain nothing special.
    // More important is whether we finally obtained the pointer or not
  }
}
Roman R.
  • 68,205
  • 6
  • 94
  • 158
  • +1 for the assert. I rarely debug code - I let the code debug itself. I have so much more free time then others who have to operate the debugger.... – jww Aug 16 '14 at 07:33
0

I think it is redundant and not needed to check both.

If it does fail though, the return value will tell you which error occurred. That's why there's 2 ways of determining if the function succeeded or not.

nemasu
  • 426
  • 2
  • 10