// 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));
Managing "Item Data" in Win32 Listbox
Author
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:
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.
Show us your SetItemData call. Also, why are your for loops
instead of just:
?
for(int i = 0; i <= nOpenFiles-1; i++)instead of just:
for(int i = 0; i < nOpenFiles; i++)?
Author
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).
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).
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:
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).
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).
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:
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);This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
