Hotkey code and text 2 clipboard merge

[Magnum]

Using XP Home Edition Version 2002 Sp2

CPU - main thread, module ntdll

EAX 00000000
ECX 77D40000 user32.77D40000
EDX 7C97C080 ntdll.7C97C080
EBX 00000000
ESP 0012FEA8
EBP 0012FFA4
ESI 7C90E88E ntdll.ZwTerminateProcess
EDI 00000000
EIP 7C90EB94 ntdll.KiFastSystemCallRet

C 0  ES 0023 32bit 0(FFFFFFFF)
P 1  CS 001B 32bit 0(FFFFFFFF)
A 0  SS 0023 32bit 0(FFFFFFFF)
Z 1  DS 0023 32bit 0(FFFFFFFF)
S 0  FS 003B 32bit 7FFDE000(FFF)
T 0  GS 0000 NULL
D 0
O 0  LastErr 00000000 ERROR_SUCCESS
EFL 00000246 (NO,NB,E,BE,NS,PE,GE,LE)

ST0 empty +UNORM 0002 00000017 00000020
ST1 empty +UNORM 001B 00000000 7FFDFC00
ST2 empty +UNORM 0045 0208002E 00000000
ST3 empty +UNORM 0003 00000000 0000002E
ST4 empty -UNORM D334 01000003 00000000
ST5 empty -UNORM FC2E 00000000 00000000
ST6 empty -UNORM D20C 0000DCE1 7C91094E
ST7 empty -UNORM D5C4 7C90EE18 0012D54C
               3 2 1 0      E S P U O Z D I
FST 0000  Cond 0 0 0 0  Err 0 0 0 0 0 0 0 0 (GT)
FCW 027F  Prec NEAR,53  Mask    1 1 1 1 1 1
Last cmnd 001B:5AD72679

XMM0 00141378 00000045 7C910732 00140000
XMM1 7C9140BB 0012D2B8 001420A0 00140000
XMM2 FFFFFFFF 7C910738 7C90EE18 0012D4FC
XMM3 00000000 7C9106EB 7C9106AB 7C910732
XMM4 001420A0 00140000 0012D808 0012D830
XMM5 7C90EE18 0012D534 0077012A 0012D2F0
XMM6 7C9106AB 7C910732 FFFFFFFF 7C910738
XMM7 0012D864 0012D870 00000000 7C9106EB
                                P U O Z D I
MXCSR 00001F80  FZ 0 DZ 0  Err  0 0 0 0 0 0
                Rnd NEAR   Mask 1 1 1 1 1 1

[dedndave]

yah - but does it work ?   :P

i have it at around 33500 cycles - lol
not a fast function to start with, i guess
111 bytes

[japheth]

No, Dave,

the MSDN docs clearly tells that you MUST NOT free the handle.
Also, it tells that you should supply an "unlocked" handle to SetClipboardData().

And IMO your code is unnecessarily hard to read (and error-prone to maintain). I'm tempted to call it a mess.

[dedndave]

thanks, Andreas   :U

it tells you not to free the handle before closing the clipboard
i cannot find the text you speak of

as for the locked/unlocked - thanks - i'll look into that

as for it being a mess - it is a matter of taste, i think
to each his own

[dedndave]

The memory must be unlocked before the Clipboard is closed.

ok - have to fix that one

[japheth]

Here's the sample copied from MSDN. Doesn't contain GlobalFree():

Code Select Expand


BOOL WINAPI EditCopy(VOID)
{
   PLABELBOX pbox;
   LPTSTR  lptstrCopy;
   HGLOBAL hglbCopy;
   int ich1, ich2, cch;

   if (hwndSelected == NULL)
       return FALSE;

   // Open the clipboard, and empty it.

   if (!OpenClipboard(hwndMain))
       return FALSE;
   EmptyClipboard();

   // Get a pointer to the structure for the selected label.

   pbox = (PLABELBOX) GetWindowLong(hwndSelected, 0);

   // If text is selected, copy it using the CF_TEXT format.

   if (pbox->fEdit)
   {
       if (pbox->ichSel == pbox->ichCaret)     // zero length
       {  
           CloseClipboard();                   // selection
           return FALSE;
       }

       if (pbox->ichSel < pbox->ichCaret)
       {
           ich1 = pbox->ichSel;
           ich2 = pbox->ichCaret;
       }
       else
       {
           ich1 = pbox->ichCaret;
           ich2 = pbox->ichSel;
       }
       cch = ich2 - ich1;

       // Allocate a global memory object for the text.

       hglbCopy = GlobalAlloc(GMEM_MOVEABLE,
           (cch + 1) * sizeof(TCHAR));
       if (hglbCopy == NULL)
       {
           CloseClipboard();
           return FALSE;
       }

       // Lock the handle and copy the text to the buffer.

       lptstrCopy = GlobalLock(hglbCopy);
       memcpy(lptstrCopy, &pbox->atchLabel[ich1],
           cch * sizeof(TCHAR));
       lptstrCopy[cch] = (TCHAR) 0;    // null character
       GlobalUnlock(hglbCopy);

       // Place the handle on the clipboard.

       SetClipboardData(CF_TEXT, hglbCopy);
   }

   // If no text is selected, the label as a whole is copied.

   else
   {
       // Save a copy of the selected label as a local memory
       // object. This copy is used to render data on request.
       // It is freed in response to the WM_DESTROYCLIPBOARD
       // message.

       pboxLocalClip = (PLABELBOX) LocalAlloc(
           LMEM_FIXED,
           sizeof(LABELBOX)
       );
       if (pboxLocalClip == NULL)
       {
           CloseClipboard();
           return FALSE;
       }
       memcpy(pboxLocalClip, pbox, sizeof(LABELBOX));
       pboxLocalClip->fSelected = FALSE;
       pboxLocalClip->fEdit = FALSE;

       // Place a registered clipboard format, the owner-display
       // format, and the CF_TEXT format on the clipboard using
       // delayed rendering.

       SetClipboardData(uLabelFormat, NULL);
       SetClipboardData(CF_OWNERDISPLAY, NULL);
       SetClipboardData(CF_TEXT, NULL);
   }

   // Close the clipboard.

   CloseClipboard();

   return TRUE;
}


as for it being a mess - it is a matter of taste, i think to each his own

Usually I'd agree, but in the Campus you'll scare newbies away with such code.

[dedndave]

now, i'm trying to find where it says the allocated block needs to be locked at all
it is in the example code, but i do not find it in the documentation text

[Magnum]

yah - but does it work ?   :P

i have it at around 33500 cycles - lol
not a fast function to start with, i guess
111 bytes

I need some more info.

What should I look for to determine if it worked?

[dedndave]

well, Andy - it has to set the text   :P
but i am at a little bit of a loss at the moment, as to whether or not GlobalLock is required
when i have more time, i will re-read the documents completely

[Gunner]

WOW, it copies a picture of a naked woman to my clipboard in Win7!!!   No, seriously, the test string gets copied to CB

[Magnum]

well, Andy - it has to set the text   :P
but i am at a little bit of a loss at the moment, as to whether or not GlobalLock is required
when i have more time, i will re-read the documents completely

It works on my system.

Andy

[dedndave]

ok - i have the bugs worked out

from MSDN...

If SetClipboardData succeeds, the system owns the object identified by the hMem  parameter. The application may not write to or free the data once
ownership has been transferred to the system, but it can lock and read from the data until the CloseClipboard function is called. (The memory must be unlocked
before the Clipboard is closed.) If the hMem parameter identifies a memory object, the object must have been allocated using the function with the
GMEM_MOVEABLE flag.

at any rate, here it is - all cleaned up and ready for testing
it is some 10,000 cycles faster, too - hopefully a sign that things are working right

test program removed - needs work

[MichaelW]

Here is another test that shows, at least under Windows 2000, no leak through a million iterations.

Code Select Expand


;====================================================================

    include \masm32\include\masm32rt.inc
    include \masm32\include\psapi.inc
    includelib \masm32\lib\psapi.lib

;====================================================================

    PROCESS_MEMORY_COUNTERS STRUCT
      cb                          DWORD ?
      PageFaultCount              DWORD ?
      PeakWorkingSetSize          DWORD ?
      WorkingSetSize              DWORD ?
      QuotaPeakPagedPoolUsage     DWORD ?
      QuotaPagedPoolUsage         DWORD ?
      QuotaPeakNonPagedPoolUsage  DWORD ?
      QuotaNonPagedPoolUsage      DWORD ?
      PagefileUsage               DWORD ?
      PeakPagefileUsage           DWORD ?
    PROCESS_MEMORY_COUNTERS ENDS

;====================================================================

testloop MACRO loopcount, freeflag
    mov ebx, loopcount
    .WHILE ebx
        invoke OpenClipboard, NULL
        .IF eax
            invoke EmptyClipboard
            invoke GlobalAlloc, GMEM_MOVEABLE, SIZEOF str1
            .IF eax
                mov hglb, eax
                invoke GlobalLock, hglb
                invoke MemCopy, ADDR str1, eax, SIZEOF str1
                invoke GlobalUnlock, hglb
                invoke SetClipboardData, CF_TEXT, hglb
                .IF eax == 0
                    print "SetClipboardData failed",13,10
                .ENDIF
            .ELSE
                print "GlobalAlloc failed",13,10
            .ENDIF
            invoke CloseClipboard
        .ELSE
            print "OpenClipboard failed",13,10
        .ENDIF
        .IF freeflag
            invoke GlobalFree, hglb
        .ENDIF
        dec ebx
    .ENDW
ENDM

;====================================================================
    .data
      hProcess dd 0
      hglb     dd 0
      pmc      PROCESS_MEMORY_COUNTERS <>
      str1     db "my other brother darryl",0
    .code
;====================================================================
start:
;====================================================================

    invoke GetCurrentProcess
    mov hProcess, eax

    print "Running 1 loop, without free:",13,10
    testloop 1, 0
    invoke GetProcessMemoryInfo, hProcess, ADDR pmc, SIZEOF pmc
    print str$(pmc.WorkingSetSize),9
    print str$(pmc.PeakWorkingSetSize),13,10
    invoke GlobalFree, hglb

    print "Running 1000000 loops, without free:",13,10
    testloop 1000000, 0
    invoke GetProcessMemoryInfo, hProcess, ADDR pmc, SIZEOF pmc
    print str$(pmc.WorkingSetSize),9
    print str$(pmc.PeakWorkingSetSize),13,10
    invoke GlobalFree, hglb

    print "Running 1 loop, with free:",13,10
    testloop 1, 1
    invoke GetProcessMemoryInfo, hProcess, ADDR pmc, SIZEOF pmc
    print str$(pmc.WorkingSetSize),9
    print str$(pmc.PeakWorkingSetSize),13,10

    print "Running 1000000 loops, with free:",13,10
    testloop 1000000, 1
    invoke GetProcessMemoryInfo, hProcess, ADDR pmc, SIZEOF pmc
    print str$(pmc.WorkingSetSize),9
    print str$(pmc.PeakWorkingSetSize),13,10

    inkey "Press any key to exit..."
    exit

;====================================================================
end start


Code Select Expand


Running 1 loop, without free:
917504  917504
Running 1000000 loops, without free:
921600  921600
Running 1 loop, with free:
921600  921600
Running 1000000 loops, with free:
921600  921600
Press any key to exit...


[dedndave]

i get some odd results when testing 100,000 iterations:

if i use GlobalFree, RuntimeChecker shows clean results - no leaks
if i do not use GlobalFree, RuntimeChecker shows 2 occurances of 152 bytes total
the unterminated string is 11 bytes - the routine allocates 11+65=76 per allocation

i am not sure how to interpret this information
on the one hand, i get no leaks and no errors by doing it the way i think is correct - lol
on the other hand, i would expect 100,000 occurances the other way

in any case, using GlobalFree does not seem to have any detrimental effects
tomorrow, i may see if i can make it do bad things
like - using GlobalFree, then trying to allocate a large block and seeing if the clipboard data is still intact
i would think that using GlobalFree when we shouldn't it should show itself

[dedndave]

ok
i managed to break things by using global free
however, if SetClipboardData fails, then the allocation should be freed
so, i have changed the order of things to...

Code Select Expand

open clipboard NULL
* if error, goto x2
global alloc GMEM_MOVEABLE or GMEM_DDESHARE
* if error, goto x1
empty clipboard
global lock
copy text to mem pointer
global unlock
set clipboard data CF_TEXT
* if no error, goto x1
global free
x1:
close clipboard
x2:

the asterisks indicate error checks

return:
0 success
1 set clipboard data failed
2 global alloc failed
3 open clipboard failed