CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 9 of 9
  1. #1
    Join Date
    Feb 2013
    Posts
    22

    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;
    }

  2. #2
    2kaud's Avatar
    2kaud is offline Super Moderator Power Poster
    Join Date
    Dec 2012
    Location
    England
    Posts
    7,822

    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)

  3. #3
    GCDEF is offline Elite Member Power Poster
    Join Date
    Nov 2003
    Location
    Florida
    Posts
    12,635

    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.

  4. #4
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,626

    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())

  5. #5
    Join Date
    Aug 2000
    Location
    New York, NY, USA
    Posts
    5,656

    Re: add to listbox thread

    Quote Originally Posted by tcp1 View Post
    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...

  6. #6
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,626

    Re: add to listbox thread

    Quote Originally Posted by VladimirF View Post
    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).

  7. #7
    Join Date
    Aug 2000
    Location
    New York, NY, USA
    Posts
    5,656

    Re: add to listbox thread

    Quote Originally Posted by OReubens View Post
    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...

  8. #8
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,626

    Re: add to listbox thread

    Quote Originally Posted by VladimirF View Post
    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).

  9. #9
    Join Date
    Feb 2003
    Location
    Iasi - Romania
    Posts
    8,234

    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.
    Ovidiu
    "When in Rome, do as Romans do."
    My latest articles: https://codexpertro.wordpress.com/

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
  •  





Click Here to Expand Forum to Full Width

Featured