Search code examples
c++visual-c++mfccode-analysis

Using std::make_unique with the GetProfileBinary function call


I have seen this answer (Advantages of using std::make_unique over new operator) where it states:

Don't use make_unique if you need a custom deleter or are adopting a raw pointer from elsewhere.

This is is my code:

void CAutomaticBackupSettingsPage::GetLastBackupDate(COleDateTime& rBackupDate)
{
    DATE* pDatTime = nullptr;
    UINT uSize;

    theApp.GetProfileBinary(_T("Options"), _T("BackupLastBackupDate"), pointer_cast<LPBYTE*>(&pDatTime), &uSize);
    if (uSize == sizeof(DATE))
        rBackupDate = *pDatTime;
    else
        rBackupDate = COleDateTime::GetCurrentTime();

    delete[] pDatTime;
    pDatTime = nullptr;
}

Code analysis gives me two warnings:

enter image description here

and

enter image description here

The latter warning suggests I use std::make_unique but since my pointer data is returned from the GetProfileBinary call, and given the statement in the related question, does that mean I should not use std::make_unique? I admit it is something I have not done before.


The useage of GetProfileBinary clearly states:

GetProfileBinary allocates a buffer and returns its address in *ppData. The caller is responsible for freeing the buffer using delete[].


Solution

  • pDateTime is supposed to be nullptr, and GetProfileBinary handles the allocation. Code Analysis mistakenly thinks you forgot the allocation.

    It does need to check for success before calling delete[]. We can't use delete[]pDatTime because pDatTime is not an array. But GetProfileBinary allocates using new BYTE[size], so we need to cast back to BYTE.

    You can also add a NULL check before reading pDatTime, that might make Code Analysis happy.

    if (pDatTime && uSize == sizeof(DATE))
        rBackupDate = *pDatTime;
    else
        rBackupDate = COleDateTime::GetCurrentTime();
    if(pDatTime) delete[](BYTE*)pDatTime;
    

    You can use std::unique_ptr<BYTE[]> cleanup((BYTE*)pDatTime) for deletion, but this has to be after GetProfileBinary is called.

    Example:

    DATE* pDatTime = nullptr;
    GetProfileBinary(_T("Options"), _T("BackupLastBackupDate"), (LPBYTE*)(&pDatTime), &uSize);
    std::unique_ptr<BYTE[]> cleanup((BYTE*)pDatTime); //automatic delete
    
    if (pDatTime && uSize == sizeof(DATE))
        rBackupDate = *pDatTime;
    else
        rBackupDate = COleDateTime::GetCurrentTime();
    
    //pDatTime = NULL; <- Error when used with unique_ptr
    ...
    //pDatTime is deleted later, when `cleanup` goes out of scope