# Managing "Item Data" in Win32 Listbox

## Recommended Posts

So a while ago I posted about an access violation where I couldn't locate the source of the problem. I apologize in advance if it's considered bad to create a new topic, but I feel this is more focused now that I know the location of the problem :) Here's what I'm doing conceptually. I'm trying to create a dialog that manages files in my source control. The dialog has 2 list boxes, files open for edit, and files that are not. When this dialog receives a WM_SHOW, I respond to it by - Assigning values to static text labels. - Freeing memory used by list box items - Delete all listbox items. - Read files in client directory, and list them in list boxes with their full path stored as item data. If I comment out the lines that should be "Freeing memory used by list box items", then my access violation crash disappears - though I get a memory leak in its place, so I do not consider it a real fix. These are the lines in question:
    // Free item data for list box items.
int nOpenFiles = ListBox_GetCount(GetDlgItem(m_hDlg, IDC_LIST_OPENFOREDIT));
int nNotOpenFiles = ListBox_GetCount(GetDlgItem(m_hDlg, IDC_LIST_NOTOPEN));

if(nOpenFiles > 0)
{
for(int i = 0; i <= nOpenFiles-1; i++)
{
wchar_t* pItemData = reinterpret_cast<wchar_t*> ( ListBox_GetItemData(GetDlgItem(m_hDlg, IDC_LIST_OPENFOREDIT), i));
delete[] pItemData;
pItemData = NULL;
}
}

if(nNotOpenFiles > 0)
{
for(int i = 0; i <= nNotOpenFiles-1; i++)
{
wchar_t* pItemData = reinterpret_cast<wchar_t*> ( ListBox_GetItemData(GetDlgItem(m_hDlg, IDC_LIST_NOTOPEN), i));
delete[] pItemData;
pItemData = NULL;
}
}

// Clear the list boxes
ListBox_ResetContent(GetDlgItem(m_hDlg, IDC_LIST_OPENFOREDIT));
ListBox_ResetContent(GetDlgItem(m_hDlg, IDC_LIST_NOTOPEN));


Can anyone see a problem in that? I haven't used a windows control's "item data" this way and I'm afraid I'm not understanding something. I'd be happy to post more code, but I don't want to flood obviously. These commented out make the problem go away, so I figure the problem is here. It doesn't crash everytime though. Only if there's a lot of files. Even then, it's not 100%, and the crash is not on these lines.

##### Share on other sites
for(int i = 0; i <= nOpenFiles-1; i++)

for(int i = 0; i < nOpenFiles; i++)

?

##### Share on other sites
Ok I guess here's where it get's tricky :/ I have a structure to store info about a file, a file stat (if you've used perforce, thats what I mean).

struct SP4FileStat{  int nClientRev;  int nHeadRev;  char szAction[LISTITEM_SIZE];  char szOwner[LISTITEM_SIZE];  char szClientFile[MAX_PATH];  char szShortFile[LISTITEM_SIZE];  SP4FileStat()  {    SecureZeroMemory(szClientFile, MAX_PATH);    SecureZeroMemory(szShortFile, LISTITEM_SIZE);    SecureZeroMemory(szAction, LISTITEM_SIZE);    SecureZeroMemory(szOwner, LISTITEM_SIZE);      }  SP4FileStat(const SP4FileStat& fs)  {    nClientRev = fs.nClientRev;    nHeadRev = fs.nHeadRev;    strcpy_s(szClientFile, MAX_PATH, fs.szClientFile);    strcpy_s(szShortFile, LISTITEM_SIZE, fs.szShortFile);    strcpy_s(szAction, LISTITEM_SIZE, fs.szAction);    strcpy_s(szOwner, LISTITEM_SIZE, fs.szOwner);  }};

I have a function you call to get file stats for all files with a certain extension in a directory. It stores it in a list (P4Manager::std::list<SP4FileStat> m_listFileStats).

void P4Manager::GetFileStatsWithoutFstat(char* szDirectory, char* szExtension){  WIN32_FIND_DATAA FindData;  char szSearchTerm[MAX_PATH];  sprintf_s(szSearchTerm, MAX_PATH, "%s\\*.%s", szDirectory, szExtension);  HANDLE hFile = FindFirstFileA(szSearchTerm, &FindData);  if(hFile == INVALID_HANDLE_VALUE) return;  do  {    SP4FileStat fs;    fs.nClientRev = 0;    fs.nHeadRev = 0;    strcpy_s(fs.szOwner, LISTITEM_SIZE, "");    strcpy_s(fs.szAction, LISTITEM_SIZE, "");    sprintf_s(fs.szShortFile, MAX_PATH, "%s", FindData.cFileName);    sprintf_s(fs.szClientFile, MAX_PATH, "%s\\%s", szDirectory, FindData.cFileName);    m_listFileStats.push_back(fs);  } while(FindNextFileA(hFile, &FindData));  FindClose(hFile);}

After that populatees the list, this function is called to put short file names (from fstats) into a listbox with their full path as item data:

void P4Manager::AddFilesToDialogWithoutFstat(HWND hDlg){  for(list<SP4FileStat>::iterator iter = m_listFileStats.begin();      iter != m_listFileStats.end();      iter++)  {    if(iter->nClientRev < 0)      continue;    // Create a string to add to the list box with file name and stats.    char szListBoxLine[LISTITEM_SIZE];    sprintf_s(szListBoxLine, LISTITEM_SIZE, "%s", iter->szShortFile);    // Determine list box to add to based on file's open for edit status.    HWND hListBox;    if(IsFileOpenForEdit(iter->szClientFile))      hListBox = GetDlgItem(hDlg, IDC_LIST_OPENFOREDIT);    else      hListBox = GetDlgItem(hDlg, IDC_LIST_NOTOPEN);    // Add string and set item data to full file path.    int nIndex = SendMessageA(hListBox, LB_ADDSTRING, 0, (LPARAM)szListBoxLine);    wchar_t* wszFilePathW = new wchar_t[MAX_PATH];    int n = MAX_PATH * sizeof(wchar_t);    SecureZeroMemory(wszFilePathW, MAX_PATH * sizeof(wchar_t));    MultiByteToWideChar(CP_ACP, 0, iter->szClientFile, -1, wszFilePathW, MAX_PATH);    ListBox_SetItemData(hListBox, nIndex, wszFilePathW);  }}

The weirdest thing I can see is that I am adding ANSI filenames with Unicode file paths as item data, but that shouldn't matter if the reinterpret_cast is working properly, right? I think item data is just a void*. As for the funny "WithoutFstat" names, just read it as "offline". These functions retrieve as much info about a file as they can if the computer is offline (which is just the filename and path).

##### Share on other sites
I only glanced briefly, but I didn't see anything weird about your new+SetItemData vs your delete+GetItemData. The only thing I can see is that you are invoking undefined behavior since the List Box will still be storing pointers to deleted memory when you call ResetContent (but I doubt this is the problem).

You are also ignoring return values during your SetItemData. I would catch and inspect the return values from the last two lines:

// Add string and set item data to full file path.    int nIndex = SendMessageA(hListBox, LB_ADDSTRING, 0, (LPARAM)szListBoxLine);    wchar_t* wszFilePathW = new wchar_t[MAX_PATH];    int n = MAX_PATH * sizeof(wchar_t);    SecureZeroMemory(wszFilePathW, MAX_PATH * sizeof(wchar_t));    MultiByteToWideChar(CP_ACP, 0, iter->szClientFile, -1, wszFilePathW, MAX_PATH);    ListBox_SetItemData(hListBox, nIndex, wszFilePathW);

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628275
• Total Posts
2981756

• 10
• 11
• 17
• 10
• 9