Sign in to follow this  
Rhaal

Managing "Item Data" in Win32 Listbox

Recommended Posts

Rhaal    754
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
mfawcett    373
Show us your SetItemData call. Also, why are your for loops

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

instead of just:

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

?

Share this post


Link to post
Share on other sites
Rhaal    754
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
mfawcett    373
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