Search code examples
visual-c++mfcsmart-pointerscode-analysis

How do I save the result from GetProfileBinary into a smart pointer?


At the moment I have the following member variable in a class:

BYTE *m_pbyImportColumnMappings;

In one of the classes we attempt to read existing data from the registry, and if it is not present, we allocate it. So far, I have changed it like this:

void CImportOCLMAssignmentHistoryDlg::ReadSettings()
{
    UINT uSize;

    m_dwImportFlags = theApp.GetNumberSetting(theApp.GetActiveScheduleSection(_T("Options")),
        _T("ImportFlags"), ImportAssignment::None);

    theApp.GetProfileBinary(theApp.GetActiveScheduleSection(_T("Options")), 
        _T("ImportColumnMappings"), (LPBYTE*)&m_pbyImportColumnMappings, &uSize);
    // Reset memory buffer (if required)
    if (uSize != (sizeof(BYTE) * 15))
    {
        if (uSize > 0)
        {
            delete[] m_pbyImportColumnMappings;
            m_pbyImportColumnMappings = nullptr;
        }

        m_pbyImportColumnMappings = new BYTE[15];
        // Default values
        const gsl::span column_mappings(m_pbyImportColumnMappings, 15);
        std::fill(begin(column_mappings), end(column_mappings), -1);
        /*
        m_pbyImportColumnMappings[0] = -1;
        m_pbyImportColumnMappings[1] = -1;
        m_pbyImportColumnMappings[2] = -1;
        m_pbyImportColumnMappings[3] = -1;
        m_pbyImportColumnMappings[4] = -1;
        m_pbyImportColumnMappings[5] = -1;
        m_pbyImportColumnMappings[6] = -1;
        m_pbyImportColumnMappings[7] = -1;
        m_pbyImportColumnMappings[8] = -1;
        m_pbyImportColumnMappings[9] = -1;
        m_pbyImportColumnMappings[10] = -1;
        m_pbyImportColumnMappings[11] = -1;
        m_pbyImportColumnMappings[12] = -1;
        m_pbyImportColumnMappings[13] = -1;
        m_pbyImportColumnMappings[14] = -1;
        */

    }
}

My initial change was to use a gsl::span to suppress several warnings about using pointer arithemetic. But I don't know how to turn m_pbyImportColumnMappings into a smart pointer, given the fact that we are attempting to initially populate it from GetProfileBinary.

If I could turn it into a smart pointer then I would not need to deallocate the memory when the class goes out of scope.

In a related answer this code was suggested:

theApp.GetProfileBinary(strSection, strEntry,
        reinterpret_cast<LPBYTE*>(&pALI), &uBytesRead);
std::unique_ptr<BYTE[]> cleanup(reinterpret_cast<BYTE*>(pALI)); 

But, I am not sure how to apply that cleanup method given teh fact we are dealing with a member variable of the class as opposed to an isolated variable in a function.


Solution

  • For a cleaner code, consider using std::vector and a temporary buffer

    std::vector<BYTE> m_mapping;
    m_mapping.resize(15, -1);
    ...
    
    UINT len = 0;
    BYTE* temp = nullptr;
    AfxGetApp()->GetProfileBinary(_T("setting"), _T("key"), &temp, &len);
    std::unique_ptr<BYTE[]> cleanup(temp);
    if (len == m_mapping.size() * sizeof(m_mapping[0]))
        memcpy(m_mapping.data(), temp, len);
    else
        std::fill(m_mapping.begin(), m_mapping.end(), -1);
    

    std::vector also has automatic cleanup and additional methods.


    Otherwise, using std::unique_ptr to replace new/delete for this member data, can be a bit of a nightmare. Example:

    m_mapping = nullptr;
    GetProfileBinary("setting", "key", &m_mapping, &uSize);
    if (uSize != (sizeof(BYTE) * 15))
    {
        { std::unique_ptr<BYTE[]> cleanup(m_mapping); }
        //delete memory immediately after exiting scope
        //note the extra brackets
    
        //allocate new memory and don't manage it anymore
        m_mapping = std::make_unique<BYTE[]>(15).release();
        if(m_mapping)
            for (int i = 0; i < 15; i++) m_mapping[i] = -1;
    }
    

    Here we are not able to take advantage of std::unique_ptr memory management, it's only used to turn off warnings.

    You don't need any casting here because it just happens that m_pbyImportColumnMappings is BYTE, and GetProfileBinary expects BYTE, it allocates memory using new BYTE