Re: add to listbox thread
Where does buf come from in ::dump()?
sprintf_S() takes as argument 2 the size of the buffer used in argument 1
Why are you passing a pointer to a window handle to Dump rather than just the window handle itself?
Where do you free the memory allocated with new in PostMessage?
AddString() can take a CString type as an argument.
One easy way to avoid the new() is to use a 2 dimensional static array in Dump (assuming that Dump() is only being used by 1 thread at once!).
A possible code re-working could be (not tried!)
Code:
void CMyDlg::startthread(LPVOID lparam)
{
AfxBeginThread(Dump, (LPVOID)GetSafeHwnd());
}
UINT CMyDlg::Dump(LPVOID lparam)
{
static char buffer[100][200];
for (int i = 0; i < 100; i++)
{
sprintf_s(buffer[i] ,200, "user %d, data %s",i, "Connected");
::PostMessage((HWND)lparam, UWM_ADD_To_LBox, 0, (LPARAM)buffer[i]);
}
}
LRESULT CMyDlg::OnAddToListBox(WPARAM wParam,LPARAM lParam)
{
m_listbox.AddString((LPCTSTR)lParam);
m_listbox.RedrawWindow();
return 0;
}
Re: add to listbox thread
Rather than a thread, you may want to use a Virtual List Control, or at least set SetRedraw to FALSE while you're loading it.
Re: add to listbox thread
a) you should not directly interact with the UI from a worker thread.
so yes, you need an asynchronous method. PostMessage is the easiest way.
though that doesn't mean strings, you can post other stuff as well.
b) OnAddToListBox doesn't leak by itself
but the new CString you created in the Dump() never gets deleted. Since Dump can't delete it, the easy place would be OnAddToListBox, though there are other possible alternatives.
c) there's little you can do with m_pThread in a reliable way as you have it here.
You typically want to:
- create the thread in suspended state
- once created, change the m_bAutoDelete to false
- Resume the thread.
you will now need to delete the m_pThread, but you can control where that happens, and you can use m_pThread->m_hThread to check if the thread ended.
Without the above, by the time you try to use m_pThread, it may already have been deleted. (and yes, that can happen even before you get the return value from AfxBeginThread())
Re: add to listbox thread
Quote:
Originally Posted by
tcp1
startthread get called to launch to update list box.
Your "magic" with HWND was already commented on.
You start with char* in your Dump(), and you need LPCTSTR for m_listbox.AddString(); wrapping it into CString just to attach to a message is absolutely unnecessary. Just use strdup().
Also, you cast it to WPARAM while passing as LPARAM. Those are NOT the same types.
The proper place to release memory attached to a posted message is in the handler; for strdup'ed string it's just a call to free().
Alternatively (as was also suggested above), you could simply use two numeric values (WPARAM and LPARAM), and format the text when needed (right before AddString()). But I sense that this is simply an example...
Why do you call RedrawWindow()? Doesn't listbox repaint itself after strings are added?
Re: add to listbox thread
Quote:
Originally Posted by
VladimirF
wrapping it into CString just to attach to a message is absolutely unnecessary. Just use strdup().
unnecessary... maybe... But I'd prefer it to strdup/free, neither of which should ideally be anywhere in a C++ application.
wrapping it in a vector<char> or std::string would be equally acceptable alternatives (and preferable to strdup/free).
Re: add to listbox thread
Quote:
Originally Posted by
OReubens
unnecessary... maybe... But I'd prefer it to strdup/free, neither of which should ideally be anywhere in a C++ application.
I would agree with you, 100%. Kinda… If we were comparing these two scenarios:
Code:
char* buffer = new char[some_length];
sprint_s(…);
and
Code:
CString str;
Str.Format(…);
Mainly for two reasons: the storage in CString will grow as needed, and it will be released when goes out of scope.
Unfortunately, the code in the original post is not taking advantage from any of these benefits. Although the character buffer inside that CString will get released automagically, the CString itself need to be deleted.
Are you saying that new/delete that manage a single char buffer is better than malloc/free for that same buffer?
I just don’t see the value in that wrapping…
Also, no one mentioned yet, but the thread function must not be a class member (or must be a static class member), while its parameter is commonly used to pass the this pointer.
Re: add to listbox thread
Quote:
Originally Posted by
VladimirF
Are you saying that new/delete that manage a single char buffer is better than malloc/free for that same buffer?
I just don’t see the value in that wrapping…
I never said either solution is "better".
I just prefer to not have malloc/calloc and other "old C style" memory allocation to pop up in my C++ code (even if the underlying library ends up doing it that way).
Re: add to listbox thread
A listbox, kept in MFC CListBox class, is a simple control designed for a reasonable number of items so generally, there is not necessary a separate thread for filling it.
For a large amount of items, use a listview control (SysListView32 kept in CListCtrl or CListView MFC class) instead.
As GCDEF already said, a listview can be made virtual then asks only for necessary data to be currently displayed. This way, avoids UI blocking without using a worker thread.
See also:
