#513 TCoolTextWnd::SetDragData doesn't consider sizeof(tchar)

[Sebastian Ledesma]

In "cooledit.cpp", the function TCoolTextWnd::SetDragData allocates memory for copying text, but the UNICODE version doesn't consider the char width, resulting in data loss.

[Sebastian Ledesma]

Solved in commit [r5725]

[Vidar Hasfjord]

Hi Sebastian, you're doing great review of the CoolPrj code. Nice work!

By the way, the logic in TCoolTextWnd::SetDragData seems unnecessary complex for the job, i.e. simply putting CF_TEXT and CF_UNICODETEXT on the clipboard. I'm sure it can be much simplified.

However, putting both formats on the clipboard isn't needed at all. Windows will provide conversions automatically:

The system performs implicit data format conversions between certain clipboard formats when an application calls the GetClipboardData function. For example, if the CF_OEMTEXT format is on the clipboard, a window can retrieve data in the CF_TEXT format. The format on the clipboard is converted to the requested format on demand. For more information, see Synthesized Clipboard Formats.

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getclipboarddata
https://docs.microsoft.com/en-us/windows/win32/dataxchg/clipboard-formats#synthesized-clipboard-formats

So you only need to retain code for the active build mode text format, and you can remove all the ugly preprocessor use.

[Vidar Hasfjord]

Also note [feature-requests:#164] "TClipboard enhancements", which makes very easy to copy text to the clipboard. Since you are targeting 7.1 for this bug fix, you may be able to utilise this new feature in your CoolPrj overhaul.

[Vidar Hasfjord]

Regarding [r5725], this part is wrong:

-    // Allocate a global memory object for the text.
-    HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, len-7);
+    // Allocate a global memory object for the converted text.
+#ifdef _UNICODE
+    HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, (len-7));
+#else
+    HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, (len-7)*sizeof(_TCHAR));
+#endif

Both of these statements evaluate to the same thing, whatever the build mode is, since if it is not a UNICODE build then sizeof(_TCHAR) == 1, so the second statement is equivalent to the first.

What you want is to allocate wide chars in ANSI build mode here. Since _TCHAR is char in ANSI build mode, you need to explicitly use wchar_t in your calculation:

HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, (len-7)*sizeof(wchar_t));

Similarly, this part is wrong:

 #ifdef _UNICODE
       memcpy(pBlock, W2A(text+7), len-7);
 #else

-      memcpy(pBlock, A2W(text+7), len-7);
+      memcpy(pBlock, A2W(text+7), (len-7)*sizeof(_TCHAR));
 #endif

Note that you want wide chars in ANSI build mode here. Since _TCHAR is char in ANSI build mode, you need to explicitly use wchar_t in your calculation:

memcpy(pBlock, A2W(text+7), (len-7)*sizeof(wchar_t));

[Sebastian Ledesma]

You are right.
It should be:

-    // Allocate a global memory object for the text.
-    HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, len-7);
+    // Allocate a global memory object for the converted text.
+#ifdef _UNICODE
+    HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, (len-7));
+#else
+    HGLOBAL hHandle2 = ::GlobalAlloc(GMEM_DDESHARE, (len-7)*sizeof(wchar_t));
+#endif

and later:

 #ifdef _UNICODE
       memcpy(pBlock, W2A(text+7), len-7);
 #else

-      memcpy(pBlock, A2W(text+7), len-7);
+      memcpy(pBlock, A2W(text+7), (len-7)*sizeof(wchar_t));
 #endif

Explanation:
In UNICODE build it will create also a CF_TEXT (ANSI) export, in ANSI it will create a CF_UNICODETEXT.

[Sebastian Ledesma]

Corrected in [r5726] .

---

Moderator: Corrected revision reference.

[Vidar Hasfjord]

TCoolTextWnd::SetDragData and other copy-and-paste functionality has been mostly rewritten in modern style in [r6548] as part of fixing [bugs:#551]. See CoolPrj code log for recent changes.