Issue reading / writing a dynamic BYTE array to the registry

Question

I have this method that initializes a buffer:

void CCreateReportDlg::InitAutoAssignStates()
{
    int iNumColumns = m_Grid.GetColumnCount();

    ASSERT(m_pbyAutoAssignStates == NULL);
    if (m_pbyAutoAssignStates == NULL)
    {
        m_pbyAutoAssignStates = new BYTE[iNumColumns];
        if (m_pbyAutoAssignStates != NULL)
        {
            // This sets them all to AUTO_ASSIGN_INCLUDE
            ZeroMemory(m_pbyAutoAssignStates, iNumColumns * sizeof(BYTE));

            // DATE is never used for auto assign
            m_pbyAutoAssignStates[COLUMN_DATE] = AUTO_ASSIGN_NOT_USED;
        }
    }
}

So far, so good. This buffer gets passed into a dialog class.

// Receives pointer to a BYTE* array.
//     This is owned by the parent.
void CAutoAssignSettingsDlg::SetAutoAssignStates(BYTE *pbyAutoAssignStates)
{
    m_pbyAutoAssignStates = pbyAutoAssignStates;
}

No problems there. I then have a checked list on the dialog that is mapped to each of the states in the above buffer.

When the popup dialog is about to close it revises the buffer:

void CAutoAssignSettingsDlg::UpdateAutoAssignStates()
{
    LVITEM  sItem;
    int     iAssign, iNumAssign;

    if (m_pbyAutoAssignStates != NULL)
    {
        sItem.mask = LVIF_IMAGE|LVIF_PARAM;
        sItem.iSubItem = 0;

        iNumAssign = m_listAssign.GetItemCount();
        for (iAssign = 0; iAssign < iNumAssign; iAssign++)
        {
            sItem.iItem = iAssign;
            m_listAssign.GetItem(&sItem);
            if (sItem.iImage == IMG_CHECKED)
                m_pbyAutoAssignStates[sItem.lParam] = AUTO_ASSIGN_EXCLUDE;
            else
                m_pbyAutoAssignStates[sItem.lParam] = AUTO_ASSIGN_INCLUDE;
        }
    }
}

This all works. But then I want to save it to the registry. At the moment I do it like this:

theApp.WriteProfileBinary(strSection, _T("AssignStates"), m_pbyAutoAssignStates, sizeof(m_pbyAutoAssignStates));

Finally, in the parent dialog, I adjusted the code that reads the settings in from the registry. So now, before the InitAutoAssignStates call I do this:

theApp.GetProfileBinary(strSection,_T("AssignStates"), &ppData, &uSize);
if (uSize > 0)
{
    m_pbyAutoAssignStates = new BYTE[uSize];
    memcpy(m_pbyAutoAssignStates, ppData, uSize);
}

// Tidy memory
if (uSize != 0)
{
    delete[] ppData;
    ppData = NULL;
}

The subsequent InitAutoAssignStates method is only called now if the buffer is NULL. So in theory I shoudlread back in the buffer that I saved. But it is not working. The set of items ticked in my check boxes do not match.

What am I doing wrong?

Andrew Truckle · Accepted Answer · 2016-11-17T16:51:33.620

0

I found a related question that said you could not do what I was trying to achieve without knowing the number of elements. This did surprise me but I am not going to argue.

I adjusted my code to pass in the number of elements to the popup dialog and then I was able to save like this:

theApp.WriteProfileBinary(strSection, _T("AssignStates"),
        m_pbyAutoAssignStates, 
        sizeof(m_pbyAutoAssignStates[0]) * m_iNumAutoAssignStateValues);

This works correctly. When I read this buffer back I get matching check boxes in my list.

edited Nov 17 '16 at 16:51

answered Nov 17 '16 at 11:10

Andrew Truckle

17,769
16
66
164

`sizeof()` returns the size of a pointer, in bytes. Pointers and arrays are different objects, where the latter can implicitly decay into the former. Since you have decided to store a pointer to the first element of the array, there's no way for your code to retrieve the array size later. It's lost. You'd need to store the length, one way or another. C++ offers container classes for this. Besides, this answer is wrong. It works by coincidence only (for 32-bit builds) and fails for 64-bit builds. – IInspectable Nov 17 '16 at 12:53
@IInspectable I am running it as 64 bit build on 64 bit PC and I do not see errors. Can you please explain? – Andrew Truckle Nov 17 '16 at 13:51
2

`sizeof(m_pbyAutoAssignStates)` is 8 for a 64-bit process. `sizeof(BYTE)` is 1 for a 64-bit process. The expression `sizeof(m_pbyAutoAssignStates) * m_iNumAutoAssignStateValues` returns 8 times as many bytes as required. That means you are writing too many bytes to the registry, which isn't as much of an issue as reading beyond the end of the array. Now that is undefined behavior, and anything can happen (including to work as desired). It's still wrong. The correct size of bytes is: `sizeof(m_pbyAutoAssignStates[0]) * m_iNumAutoAssignStateValues`. – IInspectable Nov 17 '16 at 15:04

Issue reading / writing a dynamic BYTE array to the registry

1 Answers1