Returning undocumented HRESULTS from standard COM interface methods?

Question

What is the rule on returning undocumented HRESULTS from documented standard COM objects?

i'm looking at the documentation for IObjectWithSite.SetSite, which says i should return S_OK in all situations:

Syntax
HRESULT SetSite(IUnknown *pUnkSite);

Parameters
- pUnkSite: [in] An interface pointer to the site managing this object. If NULL, the object should call IUnknown::Release to release the existing site.

Return Value
Returns S_OK in all circumstances.

Which is fine. But then i'm looking at an MSDN article detailing writing a Browser Helper Object, and there is his code for IObjectWithSite.SetSite, and it returns HRESULTS other than S_OK:

E_INVALIDARG if pUnkSite is NULL
E_INVALIDARG if pUnkSite does not support IWebBrowser2
E_POINTER if webBrowser does not support IConnectionPointContainer
result of event connection otherwise (i.e. S_OK)

HRESULT CViewSource::SetSite(IUnknown *pUnkSite)
{
  // Retrieve and store the IWebBrowser2 pointer 
  m_spWebBrowser2 = pUnkSite; 
  if (m_spWebBrowser2 == NULL)
   return E_INVALIDARG;

  // Retrieve and store the IConnectionPointerContainer pointer 
  m_spCPC = m_spWebBrowser2;
  if (m_spCPC == NULL) 
   return E_POINTER;

  // Retrieve and store the HWND of the browser. Plus install
  // a keyboard hook for further use
  RetrieveBrowserWindow();

  // Connect to the container for receiving event notifications
  return Connect();
}

The fact that the code returns E_INVALIDARG if pUnkSite is NULL looks like a bug, the documentation specifically says that the parameter can be null.

The documentation doesn't mention any other HRESULTS as possible return values - in fact it says that S_OK is to be returned in all circumstances?

Surely they can't be serious; they can't mean all circumstances. Right? What if there's an out-of-memory condition, or memory protection fault because someone pulled out a RAM stick?

What is the rule on returning undocumented HRESULTS from documented standard COM objects?

The code is violating the contract. Instead of just storing the site for later use it is using it right away. Which requires error checking. It probably works, COM programmers never ignore an HRESULT. Why don't you check to make sure. — Hans Passant
In the context of Windows Explorer and Internet Explorer loading my dll: the only call we get is IObjectWithSite.SetSite. — Ian Boyd

Simon Mourier Simon Mourier · Accepted Answer · 2011-06-27T14:59:29

When you do COM programming, you are in general very defensive. So, as a caller, you use the famous FAILED or SUCCEEDED macros (or equivalent in the language used) when calling functions of an interface.

However, when the documentation stipulates that the result is not important, it is recommended not to check the result. So, as a caller, I would do

...
myObject.SetSite(whatever);
...

instead of

...
if (FAILED(myObject.SetSite(whatever))) goto error;
...

Note this is quite specific because SetSite is documented like this.

If I don't read the doc carefully (as you did) and assume this is a "regular hresult return" interface call, I would use the FAILED macro. If you return anything undocumented, FAILED will catch it anyway, and I'm quite safe anyway.

Returning undocumented HRESULTS from standard COM interface methods?

1 Answers