0

(1) If a result of IMoniker::BindToObject is S_OK then returned pointer is valid. But PVS-Studio marks it as V595. Why?

(2) If QueryInterface is succeeded then returned pointer is valid. But PVS-Studio marks it as V595. Why?

HRESULT sc;
if (featureValidateSource)
{
    sc = lpmk->BindToObject(lpbc, NULL, IID_IOleObject, (void**)&lpObject);
    if (FAILED(sc))
    {
        lpbc->Release();
        lpmk->Release();
        if (lpObject) lpObject->Release();
        return sc;
    }
    lpObject->GetUserClassID(&clsid);                      // (1) <<< V595
}

LPOLELINK lpOleLink;
if (SUCCEEDED(m_pObject->QueryInterface(IID_IOleLink,(void**)&lpOleLink)))
{
    sc = lpOleLink->SetSourceMoniker(lpmk, clsid);         // (2) <<< V595
}

...

if (lpObject != NULL)                                      // (1) <<< V595
{
    lpObject->Update();
    lpObject->Release();
}

if (lpOleLink)                                             // (2) <<< V595
{
    lpOleLink->Release();
}
Dmitry Sokolov
  • 3,118
  • 1
  • 30
  • 35

1 Answers1

1

This is a fault of our function annotation system. PVS-Studio analyzer currently has annotations for over 6000 functions from the most popular libraries. These annotations allow the tool to perform a lot of diagnostics. However, we haven't yet annotated the functions BindToObject and QueryInterface. But even if we had, it wouldn't have helped. There are no annotations currently specifying how a returned poitner is related to the status of type HRESULT. There must be a connection: if certain functions have executed successfully, then non-null pointers are returned. There is no such thing now, but we'll gradually implement this algorithm. Then the false positives will go away.

Since there is no such connection now, the analyzer views the code in the following way:

if (X)
{
  GetPtr(&ptr1);
  if (Y)
    return;
  ptr1->foo();          // (1) <<< V595
}

if (GetPtr(&ptr2))
{
  ptr2->foo();          // (2) <<< V595
}

if (ptr1 != NULL)       // (1) <<< V595
;

if (ptr2 != NULL)       // (1) <<< V595
;

The pointers are initialized somehow. After that, they are dereferenced and then checked for NULL. It looks suspicious, so the analyzer issues the warning. So, yes, there is a fault on our part. We'll improve analysis of such constructs in time. Thank you for pointing this code out.

As for now, we suggest using one of the means of false positive suppression or use suppression base.

AndreyKarpov
  • 1,083
  • 6
  • 17