What's wrong with this code, part 13 - the answers

I loved this particular "What's wrong" because it was quite subtle (and real world).

One of the developers on the audio team came to me the other day and complained that the assert in CFooBase::~CFooBase was firing.  I looked at the source for a few minutes, and all of a sudden realized what was happening.

You see (as mirobin figured out), an object that derives from ATL::CComObjectRoot doesn't actually implement its IUnknown.  Instead, the IUnknown implementation comes from a wrapper class, the CComObject.

As a result, the IUnknown implementation in the CFooBase is dead code - it will never be called (unless someone instantiates a CFooBase directly).  But, since CFooBase implements IFooBase, it also needs to implement IUnknown (even if the implementation is never called).

What complicates this scenario is that you don't get to see the constructor for the CFooDerived class - that's hidden inside:

OBJECT_ENTRY_AUTO(CLSID_FooDerived, CFooDerived)

This macro invokes linker magic to cause the class factory for the CFooDerived to instantiate the CComObject that wraps the CFooDerived - you never see the code, so it's not at all obvious where the class is being instantiated.

So for the kudos:

First off, mad props to mirobin for initially catching two huge, massive typos (ok, stupid omissions) in the original post (I forgot the AddRef in the QI method and forgot to return a value in the Release() method).

mirobin correctly called out one other issue.  Instead of:

        if (iid == IID_FooBase)        {            AddRef();            *ppUnk = reinterpret_cast<void *>(this);        }

It should be:

        if (iid == IID_FooBase)        {            AddRef();            *ppUnk = reinterpret_cast<void *>(static_cast<IFooBase>(this));        }

The reason for this is that if CFooBase implements multiple interfaces, each interface has its own vtable.  So you need to do the double cast to resolve the ambiguity.  The good news is that the compiler will complain if there's an ambiguous cast, which forces you to resolve the ambiguity.  In this case it's not necessary, but in general it's a good idea.

Other comments: Michael Ruck complained that CFooBase() isn't implemented.  You don't have to have a constructor on a class, it's possible that this class doesn't need it.

And Michael complained that ~CFooBase() is private.  This one merits an entire blog post by itself.