bugfix: hard crash when using _ClipBoard_GetData, (from the Clipboard UDF)

[autoit@…]

This one's been around a while, and I think maybe I've fixed it. Or something. I'm sure this will help someone, since I've seen this problem go unanswered in the archived bug report forum!

I found this proposed change by looking at examples given for the Clipboard API in other programming languages, and the one *difference* that they all had in common was to lock and copy the memory when reading from the clipboard.

Func _ClipBoard_GetData_Carefully($iFormat = 1)
Local $hMemory, $tData

Local $hMemoryDest, $hLock

If Not _ClipBoard_IsFormatAvailable($iFormat) Then Return SetError(-1, 0, 0)
If Not _ClipBoard_Open(0) Then Return SetError(-2, 0, 0)
$hMemory = _ClipBoard_GetDataEx($iFormat)
_ClipBoard_Close()
If $hMemory = 0 Then Return SetError(-3, 0, 0)
Switch $iFormat
Case $CF_TEXT, $CF_OEMTEXT

;THIS WAS BAD: $tData = DllStructCreate("char Text[8192]", $hMemory)

$tData = DllStructCreate("char Text[8192]") ; do it this way so it is allocated...?
$hMemoryDest = DllStructGetPtr($tData)

; so let's lock that clipboard memory down and make our own for-sure copy.
$hLock = _MemGlobalLock($hMemory)
If $hLock = 0 Then Return SetError(-4, 0, 0)
; OK, $hLock should now be the pointer into the clipboard memory
_MemMoveMemory($hLock, $hMemoryDest, 8192)
_MemGlobalUnlock($hMemory)
; OK *now* we should have our own, good copy.

Return DllStructGetData($tData, "Text")
Case $CF_UNICODETEXT
Return _WinAPI_WideCharToMultiByte($tData)
Case Else
Return $hMemory
EndSwitch
EndFunc ;==>_ClipBoard_GetData_Carefully

[Siao]

Proposed function crashed on 1st try for me, so it's not much of a fix :) I agree that the original needs fixing though.

Case $CF_UNICODETEXT is just funny, what is it supposed to do? :) $tData is always = 0 at that point, and even if it wouldn't, _WinAPI_WideCharToMultiByte() takes pointer as param not the struct variable itself, and even if it wouldn't, why call it anyway...

As for Case $CF_TEXT and memory access error crashes,
locking/unlocking may be the textbook failsafe way to do memory operations indeed, but it's not like we're following MS recommendations 100% here anyway (for example MSDN GetClipboardData says "The application must not use the handle after the EmptyClipboard or CloseClipboard function is called", but who cares),
and I think the real problem with the code lies in the assumption of constant buffer size (8192), which is just begging for trouble. What really needs to be done is GlobalSize($hMemory) and create the struct of that returned size:

Func __ClipBoard_GetData($iFormat = 1)
	Local $hMemory, $tData, $aSize

	If Not _ClipBoard_IsFormatAvailable($iFormat) Then Return SetError(-1, 0, 0)
	If Not _ClipBoard_Open(0) Then Return SetError(-2, 0, 0)
	$hMemory = _ClipBoard_GetDataEx($iFormat)
	_ClipBoard_Close()
	If $hMemory = 0 Then Return SetError(-3, 0, 0)
	
	$aSize = DllCall('kernel32.dll','int','GlobalSize', 'ptr', $hMemory)

	Switch $iFormat
		Case $CF_TEXT, $CF_OEMTEXT
			$tData = DllStructCreate("char Text[" & $aSize[0] &"]", $hMemory)
			Return DllStructGetData($tData, "Text")
		Case $CF_UNICODETEXT
			$tData = DllStructCreate("wchar Text[" & $aSize[0]/2 &"]", $hMemory)
			Return DllStructGetData($tData, "Text")
		Case Else
			Return $hMemory
	EndSwitch

EndFunc   ;==>_ClipBoard_GetData

• Siao

[autoit@…]

I agree about allocating the actual size for the struct -- I was trying to make the smallest possible change to the existing code that would correct the problem.

But as it turns out all I did was move the problem elsewhere. :) I am still having crashes; just far, far less often, and at a different point. (The issue may even be with the other, target application's handling of the clipboard.)

Thanks for your time actually trying out my "patch". Does the original, unpatched code (from Clipboard.au3) crash for you as well? does it crash differently?

Is there an instrumented/debug build of autoit I could run in a C++ debugger? I'd be glad to help find out what's actually happening. :)

For the meantime, I will try your suggestion and see if it'll crash some entirely new way ;) ....

P.S. I'd hate to have to go back to my workaround, which was in essence implementing this function as an external program (in VBA!) and signaling it to run whenever I would have used _ClipBoard_GetData .... :-P

[autoit@…]

I have discovered that I can avoid the issue by using ClipGet() rather than _ClipBoard_GetData. It appears that the results are somewhat different when I do so -- but no crashes! I will post more as I learn it.

[autoit@…]

I think ClipGet() needs to be featured a wee bit more prominently in the help files!

[autoit@…]

It's impossible to tell for certain, but it looks as though on the occasions when _ClipBoard_GetData was crashing, ClipGet() returns an empty string, and if I then do a very brief sleep() and try ClipGet() again, I get the data I was expecting.

[Gary]

Fixed in version: 3.2.13.0