Sign in to follow this  
Rhaal

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 this post


Link to post
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 this post


Link to post
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);


Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this