What's wrong with this code, part 14

  • Article

Keeping up with the "theme" of COM related bad code examples, here's another real-world example.  To avoid any ATL confusion, it's 100% pure C++.

Our test team rolled out a new set of tests last week and immediately hit this one.  The funny thing is that the code in question had worked  without flaw for two years before this.

Obviously this has been abstracted to the point of ridiculousness, there's just enough here to demonstrate the bug...

struct FooConfig{    int _Value1;    int _Value2;};[     object,     uuid("0A0DDEDC-C422-4BB3-9869-4FED020B66C5"),     pointer_default(unique) ] __interface IFoo : IUnknown{    HRESULT GetFooConfig([out] struct FooConfig **ReturnedFooConfig);};FooConfig _GlobalFooConfig = { 1, 2};class CFoo: public IFoo{    LONG _refCount;public:    CFoo() : _refCount(1) {};    // IFoo    HRESULT GetFooConfig(FooConfig **ReturnedFooConfig)    {        *ReturnedFooConfig = &_GlobalFooConfig;        return S_OK;``    }    // IUnknown    virtual HRESULT STDMETHODCALLTYPE QueryInterface(const IID& iid, void** ppUnk)    {        HRESULT hr=S_OK;        *ppUnk = NULL;        if (iid == __uuidof(IFoo))        {            AddRef();            *ppUnk = reinterpret_cast<void *>(static_cast<IFoo *>(this));        }        else if (iid == IID_IUnknown)        {            AddRef();            *ppUnk = reinterpret_cast<void *>(static_cast<IUnknown *>(this));        }        else        {            hr = E_NOINTERFACE;        }        return hr;    }    virtual ULONG STDMETHODCALLTYPE AddRef(void)    {        return InterlockedIncrement(&_refCount);    }    virtual ULONG STDMETHODCALLTYPE Release(void)    {        LONG refCount;        refCount = InterlockedDecrement(&_refCount);        if (refCount == 0)        {            delete this;        }        return refCount;    }};

This one's pretty straighforward, and I expect that people will see the problem right away.  So I'm going to raise the bar a bit - to get full credit, you not only have to explain not only what the problem is, but also why we'd never seen a problem with this code before.

And, as always, kudos and mea culpas on Monday.

Edit: Fixed return value from GetFooConfig.