-
February 15th, 2015, 09:06 PM
#1
add to listbox thread
startthread get called to launch to update list box.
a)is it the only way to pass string to the PostMessage?. my concern is that allocate using new in each iteration each time slows it down a bit.
b)does OnAddToListBox look memory leak free?.
c)What is the best way handle m_pThread at the end?. Anyway to improve it?.
Code:
UINT CMyDlg::Dump(LPVOID lparam)
{
HWND *pHndl = static_cast<HWND *>(lparam);
char buffer[200];
for (int i = 0; i < 100; i++)
{
sprintf_s(buf,"user %d, data %s",i, "Connected");
::PostMessage(*pHndl, UWM_ADD_To_LBox, 0, (WPARAM)(new CString(buffer)));
}
delete pHndl;
}
void CMyDlg::startthread(LPVOID lparam)
{
HWND *pHndl = new HWND;
*pHndl = GetSafeHwnd();
m_pThread = AfxBeginThread(Dump, pHndl);
}
LRESULT CMyDlg::OnAddToListBox(WPARAM wParam,LPARAM lParam)
{
std::auto_ptr<CString> text(reinterpret_cast<CString*>(lParam));
m_listbox.AddString(*text);
m_listbox.RedrawWindow();
return 0;
}
-
February 16th, 2015, 07:43 AM
#2
Re: add to listbox thread
Where does buf come from in :ump()?
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;
}
Last edited by 2kaud; February 16th, 2015 at 07:45 AM.
All advice is offered in good faith only. All my code is tested (unless stated explicitly otherwise) with the latest version of Microsoft Visual Studio (using the supported features of the latest standard) and is offered as examples only - not as production quality. I cannot offer advice regarding any other c/c++ compiler/IDE or incompatibilities with VS. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/ and can be used without reference or acknowledgement. Also note that I only provide advice and guidance via the forums - and not via private messages!
C++23 Compiler: Microsoft VS2022 (17.6.5)
-
February 16th, 2015, 08:12 AM
#3
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.
-
February 16th, 2015, 08:24 AM
#4
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())
-
February 17th, 2015, 04:01 PM
#5
Re: add to listbox thread
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?
Vlad - MS MVP [2007 - 2012] - www.FeinSoftware.com
Convenience and productivity tools for Microsoft Visual Studio:
FeinWindows - replacement windows manager for Visual Studio, and more...
-
February 18th, 2015, 07:46 AM
#6
Re: add to listbox thread
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).
-
February 18th, 2015, 02:20 PM
#7
Re: add to listbox thread
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.
Vlad - MS MVP [2007 - 2012] - www.FeinSoftware.com
Convenience and productivity tools for Microsoft Visual Studio:
FeinWindows - replacement windows manager for Visual Studio, and more...
-
February 19th, 2015, 07:36 AM
#8
Re: add to listbox thread
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).
-
February 19th, 2015, 08:53 PM
#9
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:
Last edited by ovidiucucu; February 19th, 2015 at 08:56 PM.
Tags for this Thread
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|