leak using Clipboard in Unicode with richedit
in [MFC]
|
From: Barnabé on 12 Dec 2006 06:20 Hello, I am trying to use the clipboard in Unicode with richedit control , I can't get rid of a memory leak. My project is ATL with Mfc support Here is the clipboard function: HGLOBAL hBuf; _TCHAR* pBuf; CString oDataConverted = CStringSelectedinRichEdit(); oDataConverted.FreeExtra(); int iLength = (oDataConverted.GetAllocLength()+1)*sizeof(_TCHAR); // open clipboard if ( OpenClipboard() ) { hBuf = GlobalAlloc( GMEM_MOVEABLE | GMEM_ZEROINIT, iLength); pBuf = (_TCHAR*)GlobalLock(hBuf); //copy data to clipboard memcpy(pBuf,(LPCTSTR)oDataConverted,iLength); GlobalUnlock(hBuf); // remove the current clipboard contents EmptyClipboard(); SetClipboardData(CF_UNICODETEXT,hBuf); CloseClipboard(); } I check in google group for quite a while and I couldn't find the answer. The leak seem to be related to the pBuf release. The memory should be released automaticaly but somehow it's not the case. Thanks.
From: Joseph M. Newcomer on 12 Dec 2006 11:54 See below... On 12 Dec 2006 03:20:07 -0800, "Barnab�" <eric.franc(a)gmail.com> wrote: >Hello, > >I am trying to use the clipboard in Unicode with richedit control , I >can't get rid of a memory leak. >My project is ATL with Mfc support > >Here is the clipboard function: > > HGLOBAL hBuf; > _TCHAR* pBuf; > CString oDataConverted = CStringSelectedinRichEdit(); > oDataConverted.FreeExtra(); **** I see no need for FreeExtra **** > > int iLength = (oDataConverted.GetAllocLength()+1)*sizeof(_TCHAR); **** Why GetAllocLength()? Why not just GetLength()? What does allocated space have to do with the problem? **** > > // open clipboard > if ( OpenClipboard() ) > { > hBuf = GlobalAlloc( GMEM_MOVEABLE | GMEM_ZEROINIT, iLength); > pBuf = (_TCHAR*)GlobalLock(hBuf); > //copy data to clipboard > memcpy(pBuf,(LPCTSTR)oDataConverted,iLength); > GlobalUnlock(hBuf); > > // remove the current clipboard contents > EmptyClipboard(); > SetClipboardData(CF_UNICODETEXT,hBuf); > CloseClipboard(); > } > >I check in google group for quite a while and I couldn't find the >answer. >The leak seem to be related to the pBuf release. >The memory should be released automaticaly but somehow it's not the >case. > **** There had better not be any release of that storage! It is now owned by the clipboard, and you must not touch it. What is reporting this as a leak? Note that a lot of tools notice you did not call GlobalFree, and therefore would erroneously report that this storage had "leaked", when in fact the program is absolutely correct. A tool that reports a leak of storage that has been set in the clipboard is reporting a nonexistent leak and can be construed as being erroneous. It should be handling the SetClipboard call and treat this (logically, from its viewpoint) as a proxy for the GlobalFree operation. **** >Thanks. Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Barnabé on 13 Dec 2006 02:08 Thanks for the help, I have checked more in details what was wrong with my code and it appeared that it's not the clipboard whose making the leak but the rich editGetSelText function. Here is the code I use: CString CTest::GetSelText() { CHARRANGE cr; cr.cpMin = cr.cpMax = 0; ::SendMessage(m_hWnd, EM_EXGETSEL, 0, (LPARAM)&cr); LPSTR lpsz = (char*)_alloca((cr.cpMax - cr.cpMin + 1)*2); lpsz[0] = NULL; ::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)lpsz); return CString(lpsz); } I have also tried the following one, I think this one would be more logical as I use unicode. but for some reason it does not work at all. CString CTest::GetSelection() { CHARRANGE cr; GetRichEditCtrl().GetSel(cr); // Allocate memory for the buffer LPTSTR lptbuf = new TCHAR[cr.cpMax - cr.cpMin + 1]; // Get the selected text from the Rich Edit GetRichEditCtrl().SendMessage(EM_GETSELTEXT, 0, (LPARAM) lptbuf); CString strSelect(lptbuf); // Release allocated memory delete [] lptbuf; return strSelect; }
From: Joseph M. Newcomer on 13 Dec 2006 16:23 See below... On 12 Dec 2006 23:08:28 -0800, "Barnab�" <eric.franc(a)gmail.com> wrote: >Thanks for the help, > >I have checked more in details what was wrong with my code and it >appeared that it's not the clipboard whose making the leak >but the rich editGetSelText function. > >Here is the code I use: > >CString CTest::GetSelText() >{ > CHARRANGE cr; > cr.cpMin = cr.cpMax = 0; > ::SendMessage(m_hWnd, EM_EXGETSEL, 0, (LPARAM)&cr); **** And why doesn't GetSel(cr); do the job? This seems a clumsy way to implement what is already in MFC. **** > LPSTR lpsz = (char*)_alloca((cr.cpMax - cr.cpMin + 1)*2); **** This is unbelievably clumsy! Why do something this complex? What's the *2 doing there? Did you mean sizeof(TCHAR)? Why are you using an LPSTR buffer (which is for ASCII characters) and casting to char* if you are in Unicode? Why not LPTSTR lpsz = (LPTSTR)_alloca((cr.cpMax - cpMin + 1) * sizeof(TCHAR)); but that's still an awfully complicated way to solve a simple problem. But the REAL solution, if you have to do this (you haven't said if you are working in VS6 or VS.NET, and to do this in VS6 you need to implement it yourself) is CString s; LPTSTR p = s.GetBuffer(cr.cpMax - cr.cpMin + 1); ::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)p); s.ReleaseBuffer(); return s; and no ugly obsolete _alloca calls are required! No sizeof(TCHAR)! It is about as trivial as you can get. And it works correctly in both Unicode and ANSI versions. **** > lpsz[0] = NULL; **** NULL is a pointer value. _T('\0') is a character value. Why are you writing a pointer into a character value in a buffer that is about to be overwritten anyway? **** > ::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)lpsz); **** Again, a remarkably clumsy way to handle this! If you're using anything beyond VS6, you can just write return GetSelText(); **** > return CString(lpsz); >} **** In anything beyond VS6 this whole piece of code could be replaced simply by the GetSelText call on the CRichEditCtrl! **** > >I have also tried the following one, I think this one would be more >logical as I use unicode. >but for some reason it does not work at all. > >CString CTest::GetSelection() >{ > CHARRANGE cr; > GetRichEditCtrl().GetSel(cr); > > // Allocate memory for the buffer > LPTSTR lptbuf = new TCHAR[cr.cpMax - cr.cpMin + 1]; > > // Get the selected text from the Rich Edit > GetRichEditCtrl().SendMessage(EM_GETSELTEXT, 0, (LPARAM) lptbuf); > > CString strSelect(lptbuf); > > // Release allocated memory > delete [] lptbuf; > > return strSelect; >} Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Barnabé on 14 Dec 2006 02:50
I tried that function GetSeltext without any success I found the solution , in atl there is no CString GetSelText() but BOOL GetSelTextBSTR(BSTR& bstrText) const Here is the GetSelTextBSTR code BOOL GetSelTextBSTR(BSTR& bstrText) const { ATLENSURE(::IsWindow(m_hWnd)); ATLENSURE(bstrText == NULL); CHARRANGE cr; cr.cpMin = cr.cpMax = 0; ::SendMessage(m_hWnd, EM_EXGETSEL, 0, (LPARAM)&cr); LPSTR lpstrText = (char*)_alloca((cr.cpMax - cr.cpMin + 1) * 2); lpstrText[0] = 0; if(::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)lpstrText) == 0) return FALSE; bstrText = ::SysAllocString(CA2W(lpstrText)); return (bstrText != NULL) ? TRUE : FALSE; } They have done almost the same as me. about the "2", I agree that sizeof(TCHAR) is way more better. about the following code you gave, I have tried similar and it failed: CString s; LPTSTR p = s.GetBuffer(cr.cpMax - cr.cpMin + 1); ::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)p); s.ReleaseBuffer(); return s; EM_GETSELTEXT return incorrect characters. LPARAM return LPSTR not LPTSTR. Thanks for your precision. On Dec 13, 11:23 pm, Joseph M. Newcomer <newco...(a)flounder.com> wrote: > See below... > On 12 Dec 2006 23:08:28 -0800, "Barnabé" <eric.fr...(a)gmail.com> wrote: > > >Thanks for the help, > > >I have checked more in details what was wrong with my code and it > >appeared that it's not the clipboard whose making the leak > >but the rich editGetSelText function. > > >Here is the code I use: > > >CString CTest::GetSelText() > >{ > > CHARRANGE cr; > > cr.cpMin = cr.cpMax = 0; > > ::SendMessage(m_hWnd, EM_EXGETSEL, 0, (LPARAM)&cr);**** > And why doesn't > GetSel(cr); > do the job? This seems a clumsy way to implement what is already in MFC. > ****> LPSTR lpsz = (char*)_alloca((cr.cpMax - cr.cpMin + 1)*2);**** > This is unbelievably clumsy! Why do something this complex? What's the *2 doing there? > Did you mean sizeof(TCHAR)? Why are you using an LPSTR buffer (which is for ASCII > characters) and casting to char* if you are in Unicode? Why not > LPTSTR lpsz = (LPTSTR)_alloca((cr.cpMax - cpMin + 1) * sizeof(TCHAR)); > but that's still an awfully complicated way to solve a simple problem. But the REAL > solution, if you have to do this (you haven't said if you are working in VS6 or VS.NET, > and to do this in VS6 you need to implement it yourself) is > > CString s; > LPTSTR p = s.GetBuffer(cr.cpMax - cr.cpMin + 1); > ::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)p); > s.ReleaseBuffer(); > return s; > and no ugly obsolete _alloca calls are required! No sizeof(TCHAR)! It is about as > trivial as you can get. And it works correctly in both Unicode and ANSI versions. > ****> lpsz[0] = NULL;**** > NULL is a pointer value. _T('\0') is a character value. Why are you writing a pointer > into a character value in a buffer that is about to be overwritten anyway? > ****> ::SendMessage(m_hWnd, EM_GETSELTEXT, 0, (LPARAM)lpsz);**** > Again, a remarkably clumsy way to handle this! If you're using anything beyond VS6, you > can just write > return GetSelText(); > **** > return CString(lpsz); > >}**** > In anything beyond VS6 this whole piece of code could be replaced simply by the GetSelText > call on the CRichEditCtrl! > **** > > > > >I have also tried the following one, I think this one would be more > >logical as I use unicode. > >but for some reason it does not work at all. > > >CString CTest::GetSelection() > >{ > > CHARRANGE cr; > > GetRichEditCtrl().GetSel(cr); > > > // Allocate memory for the buffer > > LPTSTR lptbuf = new TCHAR[cr.cpMax - cr.cpMin + 1]; > > > // Get the selected text from the Rich Edit > > GetRichEditCtrl().SendMessage(EM_GETSELTEXT, 0, (LPARAM) lptbuf); > > > CString strSelect(lptbuf); > > > // Release allocated memory > > delete [] lptbuf; > > > return strSelect; > >}Joseph M. Newcomer [MVP] > email: newco...(a)flounder.com > Web:http://www.flounder.com > MVP Tips:http://www.flounder.com/mvp_tips.htm |