From ff018d88b9f0fe23ba96c81b53d58532baf4df42 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Thu, 2 Oct 2014 14:10:50 +0100 Subject: [PATCH 1/9] hw/xwin: Remove some redundant clipboard externs, now defined in winglobals.h Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboardwrappers.c | 6 ------ hw/xwin/winprocarg.c | 9 --------- 2 files changed, 15 deletions(-) diff --git a/hw/xwin/winclipboardwrappers.c b/hw/xwin/winclipboardwrappers.c index 2679f4f98..2e6b63287 100644 --- a/hw/xwin/winclipboardwrappers.c +++ b/hw/xwin/winclipboardwrappers.c @@ -43,12 +43,6 @@ DISPATCH_PROC(winProcEstablishConnection); -/* - * References to external symbols - */ - -extern Bool g_fClipboard; - /* * Wrapper for internal EstablishConnection function. * Initializes internal clients that must not be started until diff --git a/hw/xwin/winprocarg.c b/hw/xwin/winprocarg.c index f2bf05bad..f6d14a1b8 100644 --- a/hw/xwin/winprocarg.c +++ b/hw/xwin/winprocarg.c @@ -37,15 +37,6 @@ from The Open Group. #include "winmsg.h" #include "winmonitors.h" -/* - * References to external symbols - */ - -#ifdef XWIN_CLIPBOARD -extern Bool g_fUnicodeClipboard; -extern Bool g_fClipboard; -#endif - /* * Function prototypes */ From c5ad92077e7eb32590fafa92d697cc4173f7e57b Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Tue, 24 Sep 2013 12:23:42 +0100 Subject: [PATCH 2/9] hw/xwin: In SelectionNotify, delete the property containing returned data after we have retrieved it Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/xevents.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/xwin/winclipboard/xevents.c b/hw/xwin/winclipboard/xevents.c index 33d52aafd..f4212b49d 100644 --- a/hw/xwin/winclipboard/xevents.c +++ b/hw/xwin/winclipboard/xevents.c @@ -568,13 +568,13 @@ winClipboardFlushXEvents(HWND hwnd, winDebug("SelectionNotify - returned data %d left %d\n", xtpText.nitems, ulReturnBytesLeft); - /* Request the selection data */ + /* Retrieve the selection data and delete the property */ iReturn = XGetWindowProperty(pDisplay, iWindow, atomLocalProperty, 0, ulReturnBytesLeft, - False, + True, AnyPropertyType, &xtpText.encoding, &xtpText.format, From 4db1241037e3fe8f0a46888377b8fef40bae9065 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Tue, 24 Sep 2013 13:30:32 +0100 Subject: [PATCH 3/9] hw/xwin: In SelectionNotify, don't pointlessly retrieve just the size of the property Don't pointlessly retrieve just the size of the property, if we are then going to assume we can retrieve the whole property in one request anyhow... Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/xevents.c | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/hw/xwin/winclipboard/xevents.c b/hw/xwin/winclipboard/xevents.c index f4212b49d..42e9147b5 100644 --- a/hw/xwin/winclipboard/xevents.c +++ b/hw/xwin/winclipboard/xevents.c @@ -43,6 +43,7 @@ #undef _XSERVER64 #endif +#include #include "internal.h" #include #include @@ -551,29 +552,12 @@ winClipboardFlushXEvents(HWND hwnd, } } - /* Retrieve the size of the stored data */ - iReturn = XGetWindowProperty(pDisplay, iWindow, atomLocalProperty, 0, 0, /* Don't get data, just size */ - False, - AnyPropertyType, - &xtpText.encoding, - &xtpText.format, - &xtpText.nitems, - &ulReturnBytesLeft, &xtpText.value); - if (iReturn != Success) { - ErrorF("winClipboardFlushXEvents - SelectionNotify - " - "XGetWindowProperty () failed, aborting: %d\n", iReturn); - break; - } - - winDebug("SelectionNotify - returned data %d left %d\n", - xtpText.nitems, ulReturnBytesLeft); - /* Retrieve the selection data and delete the property */ iReturn = XGetWindowProperty(pDisplay, iWindow, atomLocalProperty, 0, - ulReturnBytesLeft, + INT_MAX, True, AnyPropertyType, &xtpText.encoding, From 851b50419907276fdc781b6be2d42b38849bbd77 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Tue, 24 Sep 2013 16:02:37 +0100 Subject: [PATCH 4/9] hw/xwin: Retrieve TARGETS to avoid unnecessary failing conversion attempts See http://cygwin.com/ml/cygwin-xfree/2013-07/msg00016.html It looks like the change in a9aca218f557c723e637287272819a7c17174e1e had some unforseen consequences. If the X11 selection contents are not convertable to COMPOUND_TEXT, UTF8_STRING or STRING format (for example, if it is an image), after those conversion attempts have failed, we sit in winProcessXEventsTimeout() until the timeout expires. It also seems that maybe gnuplot doesn't respond correctly to this sequence of conversion requests and doesn't reply to some of them, which also causes us to sit in winProcessXEventsTimeout() until the timeout expires. The Windows application which has requested the clipboard contents via GetClipboardContents() is blocked until we return from WM_RENDERFORMAT, so sitting waiting for this timeout to expire should be avoided. So instead, explicitly request conversion to the TARGETS target, choose the most preferred format, and request conversion to that. Also: if there is no owned selection, there is nothing to paste, so don't bother trying to convert it. v2: Fix compilation with -Werror=declaration-after-statement Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/internal.h | 13 ++- hw/xwin/winclipboard/thread.c | 6 +- hw/xwin/winclipboard/wndproc.c | 139 +++++++++++++++++++++++++------- hw/xwin/winclipboard/xevents.c | 123 ++++++++++++++++------------ 4 files changed, 195 insertions(+), 86 deletions(-) diff --git a/hw/xwin/winclipboard/internal.h b/hw/xwin/winclipboard/internal.h index 94956f80d..bcf45ca4d 100644 --- a/hw/xwin/winclipboard/internal.h +++ b/hw/xwin/winclipboard/internal.h @@ -39,8 +39,9 @@ #include #define WIN_XEVENTS_SUCCESS 0 -#define WIN_XEVENTS_CONVERT 2 -#define WIN_XEVENTS_NOTIFY 3 +#define WIN_XEVENTS_FAILED 1 +#define WIN_XEVENTS_NOTIFY_DATA 3 +#define WIN_XEVENTS_NOTIFY_TARGETS 4 #define WM_WM_REINIT (WM_USER + 1) @@ -95,9 +96,15 @@ typedef struct * winclipboardxevents.c */ +typedef struct +{ + Bool fUseUnicode; + Atom *targetList; +} ClipboardConversionData; + int winClipboardFlushXEvents(HWND hwnd, - Window iWindow, Display * pDisplay, Bool fUnicodeSupport, ClipboardAtoms *atom); + Window iWindow, Display * pDisplay, ClipboardConversionData *data, ClipboardAtoms *atom); Atom diff --git a/hw/xwin/winclipboard/thread.c b/hw/xwin/winclipboard/thread.c index c179e3f83..be053574d 100644 --- a/hw/xwin/winclipboard/thread.c +++ b/hw/xwin/winclipboard/thread.c @@ -123,6 +123,7 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) int iSelectError; Bool fShutdown = FALSE; static Bool fErrorHandlerSet = FALSE; + ClipboardConversionData data; winDebug("winClipboardProc - Hello\n"); @@ -260,7 +261,8 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) * because there may be events in local data structures * already. */ - winClipboardFlushXEvents(hwnd, iWindow, pDisplay, fUseUnicode, &atoms); + data.fUseUnicode = fUseUnicode; + winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms); /* Pre-flush Windows messages */ if (!winClipboardFlushWindowsMessageQueue(hwnd)) { @@ -318,7 +320,7 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) /* Branch on which descriptor became active */ if (FD_ISSET(iConnectionNumber, &fdsRead)) { /* Process X events */ - winClipboardFlushXEvents(hwnd, iWindow, pDisplay, fUseUnicode, &atoms); + winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms); } #ifdef HAS_DEVWINDOWS diff --git a/hw/xwin/winclipboard/wndproc.c b/hw/xwin/winclipboard/wndproc.c index 165ff558a..23a0ac9bf 100644 --- a/hw/xwin/winclipboard/wndproc.c +++ b/hw/xwin/winclipboard/wndproc.c @@ -45,6 +45,7 @@ #include #include +#include #include @@ -64,7 +65,7 @@ static int winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay, - Bool fUseUnicode, ClipboardAtoms *atoms, int iTimeoutSec) + ClipboardConversionData *data, ClipboardAtoms *atoms, int iTimeoutSec) { int iConnNumber; struct timeval tv; @@ -117,14 +118,14 @@ winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay, /* Process X events */ /* Exit when we see that server is shutting down */ iReturn = winClipboardFlushXEvents(hwnd, - iWindow, pDisplay, fUseUnicode, atoms); + iWindow, pDisplay, data, atoms); winDebug ("winProcessXEventsTimeout () - winClipboardFlushXEvents returned %d\n", iReturn); - if (WIN_XEVENTS_NOTIFY == iReturn) { - /* Bail out if notify processed */ + if ((WIN_XEVENTS_NOTIFY_DATA == iReturn) || (WIN_XEVENTS_NOTIFY_TARGETS == iReturn) || (WIN_XEVENTS_FAILED == iReturn)) { + /* Bail out */ return iReturn; } } @@ -415,6 +416,10 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) { int iReturn; Bool fConvertToUnicode; + Bool pasted = FALSE; + Atom selection; + ClipboardConversionData data; + int best_target = 0; winDebug("winClipboardWindowProc - WM_RENDER*FORMAT - Hello.\n"); @@ -424,18 +429,88 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) else fConvertToUnicode = (CF_UNICODETEXT == wParam); - /* Request the selection contents */ - iReturn = XConvertSelection(pDisplay, - winClipboardGetLastOwnedSelectionAtom(atoms), - atoms->atomCompoundText, - atoms->atomLocalProperty, - iWindow, CurrentTime); - if (iReturn == BadAtom || iReturn == BadWindow) { - ErrorF("winClipboardWindowProc - WM_RENDER*FORMAT - " - "XConvertSelection () failed\n"); - break; + selection = winClipboardGetLastOwnedSelectionAtom(atoms); + if (selection == None) { + ErrorF("winClipboardWindowProc - no monitored selection is owned\n"); + goto fake_paste; } + winDebug("winClipboardWindowProc - requesting targets for selection from owner\n"); + + /* Request the selection's supported conversion targets */ + XConvertSelection(pDisplay, + selection, + atoms->atomTargets, + atoms->atomLocalProperty, + iWindow, CurrentTime); + + /* Process X events */ + data.fUseUnicode = fConvertToUnicode; + iReturn = winProcessXEventsTimeout(hwnd, + iWindow, + pDisplay, + &data, + atoms, + WIN_POLL_TIMEOUT); + + if (WIN_XEVENTS_NOTIFY_TARGETS != iReturn) { + ErrorF + ("winClipboardWindowProc - timed out waiting for WIN_XEVENTS_NOTIFY_TARGETS\n"); + goto fake_paste; + } + + /* Choose the most preferred target */ + { + struct target_priority + { + Atom target; + unsigned int priority; + }; + + struct target_priority target_priority_table[] = + { + { atoms->atomCompoundText, 0 }, +#ifdef X_HAVE_UTF8_STRING + { atoms->atomUTF8String, 1 }, +#endif + { XA_STRING, 2 }, + }; + + int best_priority = INT_MAX; + + int i,j; + for (i = 0 ; data.targetList[i] != 0; i++) + { + for (j = 0; j < sizeof(target_priority_table)/sizeof(struct target_priority); j ++) + { + if ((data.targetList[i] == target_priority_table[j].target) && + (target_priority_table[j].priority < best_priority)) + { + best_target = target_priority_table[j].target; + best_priority = target_priority_table[j].priority; + } + } + } + } + + free(data.targetList); + data.targetList = 0; + + winDebug("winClipboardWindowProc - best target is %d\n", best_target); + + /* No useful targets found */ + if (best_target == 0) + goto fake_paste; + + winDebug("winClipboardWindowProc - requesting selection from owner\n"); + + /* Request the selection contents */ + XConvertSelection(pDisplay, + selection, + best_target, + atoms->atomLocalProperty, + iWindow, CurrentTime); + /* Special handling for WM_RENDERALLFORMATS */ if (message == WM_RENDERALLFORMATS) { /* We must open and empty the clipboard */ @@ -449,40 +524,47 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) ErrorF("winClipboardWindowProc - WM_RENDER*FORMATS - " "OpenClipboard () failed: %08x\n", GetLastError()); - break; } if (!EmptyClipboard()) { ErrorF("winClipboardWindowProc - WM_RENDER*FORMATS - " "EmptyClipboard () failed: %08x\n", GetLastError()); - break; } } - /* Process the SelectionNotify event */ + /* Process X events */ iReturn = winProcessXEventsTimeout(hwnd, iWindow, pDisplay, - fConvertToUnicode, + &data, atoms, WIN_POLL_TIMEOUT); /* - * The last call to winProcessXEventsTimeout - * from above had better have seen a notify event, or else we - * are dealing with a buggy or old X11 app. In these cases we - * have to paste some fake data to the Win32 clipboard to - * satisfy the requirement that we write something to it. + * winProcessXEventsTimeout had better have seen a notify event, + * or else we are dealing with a buggy or old X11 app. */ - if (WIN_XEVENTS_NOTIFY != iReturn) { + if (WIN_XEVENTS_NOTIFY_DATA != iReturn) { + ErrorF + ("winClipboardWindowProc - timed out waiting for WIN_XEVENTS_NOTIFY_DATA\n"); + } + else { + pasted = TRUE; + } + + /* + * If we couldn't get the data from the X clipboard, we + * have to paste some fake data to the Win32 clipboard to + * satisfy the requirement that we write something to it. + */ + fake_paste: + if (!pasted) + { /* Paste no data, to satisfy required call to SetClipboardData */ SetClipboardData(CF_UNICODETEXT, NULL); SetClipboardData(CF_TEXT, NULL); - - ErrorF - ("winClipboardWindowProc - timed out waiting for WIN_XEVENTS_NOTIFY\n"); - } + } /* Special handling for WM_RENDERALLFORMATS */ if (message == WM_RENDERALLFORMATS) { @@ -492,7 +574,6 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) ErrorF("winClipboardWindowProc - WM_RENDERALLFORMATS - " "CloseClipboard () failed: %08x\n", GetLastError()); - break; } } diff --git a/hw/xwin/winclipboard/xevents.c b/hw/xwin/winclipboard/xevents.c index 42e9147b5..3dca35496 100644 --- a/hw/xwin/winclipboard/xevents.c +++ b/hw/xwin/winclipboard/xevents.c @@ -134,13 +134,59 @@ winClipboardInitMonitoredSelections(void) lastOwnedSelectionIndex = CLIP_OWN_NONE; } +static int +winClipboardSelectionNotifyTargets(HWND hwnd, Window iWindow, Display *pDisplay, ClipboardConversionData *data, ClipboardAtoms *atoms) +{ + Atom type; + int format; + unsigned long nitems; + unsigned long after; + Atom *prop; + + /* Retrieve the selection data and delete the property */ + int iReturn = XGetWindowProperty(pDisplay, + iWindow, + atoms->atomLocalProperty, + 0, + INT_MAX, + True, + AnyPropertyType, + &type, + &format, + &nitems, + &after, + (unsigned char **)&prop); + if (iReturn != Success) { + ErrorF("winClipboardFlushXEvents - SelectionNotify - " + "XGetWindowProperty () failed, aborting: %d\n", iReturn); + } else { + int i; + data->targetList = malloc((nitems+1)*sizeof(Atom)); + + for (i = 0; i < nitems; i++) + { + Atom atom = prop[i]; + char *pszAtomName = XGetAtomName(pDisplay, atom); + data->targetList[i] = atom; + winDebug("winClipboardFlushXEvents - SelectionNotify - target[%d] %d = %s\n", i, atom, pszAtomName); + XFree(pszAtomName); + } + + data->targetList[nitems] = 0; + + XFree(prop); + } + + return WIN_XEVENTS_NOTIFY_TARGETS; +} + /* * Process any pending X events */ int winClipboardFlushXEvents(HWND hwnd, - Window iWindow, Display * pDisplay, Bool fUseUnicode, ClipboardAtoms *atoms) + Window iWindow, Display * pDisplay, ClipboardConversionData *data, ClipboardAtoms *atoms) { Atom atomClipboard = atoms->atomClipboard; Atom atomLocalProperty = atoms->atomLocalProperty; @@ -273,7 +319,7 @@ winClipboardFlushXEvents(HWND hwnd, fCloseClipboard = TRUE; /* Check that clipboard format is available */ - if (fUseUnicode && !IsClipboardFormatAvailable(CF_UNICODETEXT)) { + if (data->fUseUnicode && !IsClipboardFormatAvailable(CF_UNICODETEXT)) { static int count; /* Hack to stop acroread spamming the log */ static HWND lasthwnd; /* I've not seen any other client get here repeatedly? */ @@ -290,7 +336,7 @@ winClipboardFlushXEvents(HWND hwnd, fAbort = TRUE; goto winClipboardFlushXEvents_SelectionRequest_Done; } - else if (!fUseUnicode && !IsClipboardFormatAvailable(CF_TEXT)) { + else if (!data->fUseUnicode && !IsClipboardFormatAvailable(CF_TEXT)) { ErrorF("winClipboardFlushXEvents - CF_TEXT is not " "available from Win32 clipboard. Aborting.\n"); @@ -312,7 +358,7 @@ winClipboardFlushXEvents(HWND hwnd, xiccesStyle = XStringStyle; /* Get a pointer to the clipboard text, in desired format */ - if (fUseUnicode) { + if (data->fUseUnicode) { /* Retrieve clipboard data */ hGlobal = GetClipboardData(CF_UNICODETEXT); } @@ -331,7 +377,7 @@ winClipboardFlushXEvents(HWND hwnd, pszGlobalData = (char *) GlobalLock(hGlobal); /* Convert the Unicode string to UTF8 (MBCS) */ - if (fUseUnicode) { + if (data->fUseUnicode) { iConvertDataLen = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR) pszGlobalData, @@ -362,7 +408,7 @@ winClipboardFlushXEvents(HWND hwnd, xtpText.nitems = 0; /* Create the text property from the text list */ - if (fUseUnicode) { + if (data->fUseUnicode) { #ifdef X_HAVE_UTF8_STRING iReturn = Xutf8TextListToTextProperty(pDisplay, pszTextList, @@ -507,49 +553,22 @@ winClipboardFlushXEvents(HWND hwnd, } /* - * Request conversion of UTF8 and CompoundText targets. - */ + SelectionNotify with property of None indicates either: + + (i) Generated by the X server if no owner for the specified selection exists + (perhaps it's disappeared on us mid-transaction), or + (ii) Sent by the selection owner when the requested selection conversion could + not be performed or server errors prevented the conversion data being returned + */ if (event.xselection.property == None) { - if (event.xselection.target == XA_STRING) { - winDebug("winClipboardFlushXEvents - SelectionNotify - " - "XA_STRING\n"); - - return WIN_XEVENTS_CONVERT; - } - else if (event.xselection.target == atomUTF8String) { - winDebug("winClipboardFlushXEvents - SelectionNotify - " - "Requesting conversion of UTF8 target.\n"); - - XConvertSelection(pDisplay, - event.xselection.selection, - XA_STRING, - atomLocalProperty, iWindow, CurrentTime); - - /* Process the ConvertSelection event */ - XFlush(pDisplay); - return WIN_XEVENTS_CONVERT; - } -#ifdef X_HAVE_UTF8_STRING - else if (event.xselection.target == atomCompoundText) { - winDebug("winClipboardFlushXEvents - SelectionNotify - " - "Requesting conversion of CompoundText target.\n"); - - XConvertSelection(pDisplay, - event.xselection.selection, - atomUTF8String, - atomLocalProperty, iWindow, CurrentTime); - - /* Process the ConvertSelection event */ - XFlush(pDisplay); - return WIN_XEVENTS_CONVERT; - } -#endif - else { ErrorF("winClipboardFlushXEvents - SelectionNotify - " - "Unknown format. Cannot request conversion, " - "aborting.\n"); - break; + "Conversion to format %d refused.\n", + event.xselection.target); + return WIN_XEVENTS_FAILED; } + + if (event.xselection.target == atomTargets) { + return winClipboardSelectionNotifyTargets(hwnd, iWindow, pDisplay, data, atoms); } /* Retrieve the selection data and delete the property */ @@ -567,7 +586,7 @@ winClipboardFlushXEvents(HWND hwnd, if (iReturn != Success) { ErrorF("winClipboardFlushXEvents - SelectionNotify - " "XGetWindowProperty () failed, aborting: %d\n", iReturn); - break; + goto winClipboardFlushXEvents_SelectionNotify_Done; } { @@ -581,7 +600,7 @@ winClipboardFlushXEvents(HWND hwnd, pszAtomName = NULL; } - if (fUseUnicode) { + if (data->fUseUnicode) { #ifdef X_HAVE_UTF8_STRING /* Convert the text property to a text list */ iReturn = Xutf8TextPropertyToTextList(pDisplay, @@ -648,7 +667,7 @@ winClipboardFlushXEvents(HWND hwnd, /* Convert the X clipboard string to DOS format */ winClipboardUNIXtoDOS(&pszReturnData, strlen(pszReturnData)); - if (fUseUnicode) { + if (data->fUseUnicode) { /* Find out how much space needed to convert MBCS to Unicode */ iUnicodeLen = MultiByteToWideChar(CP_UTF8, 0, @@ -707,7 +726,7 @@ winClipboardFlushXEvents(HWND hwnd, } /* Copy the returned string into the global memory */ - if (fUseUnicode) { + if (data->fUseUnicode) { memcpy(pszGlobalData, pwszUnicodeStr, sizeof(wchar_t) * (iUnicodeLen + 1)); free(pwszUnicodeStr); @@ -724,7 +743,7 @@ winClipboardFlushXEvents(HWND hwnd, pszGlobalData = NULL; /* Push the selection data to the Windows clipboard */ - if (fUseUnicode) + if (data->fUseUnicode) SetClipboardData(CF_UNICODETEXT, hGlobal); else SetClipboardData(CF_TEXT, hGlobal); @@ -754,7 +773,7 @@ winClipboardFlushXEvents(HWND hwnd, SetClipboardData(CF_UNICODETEXT, NULL); SetClipboardData(CF_TEXT, NULL); } - return WIN_XEVENTS_NOTIFY; + return WIN_XEVENTS_NOTIFY_DATA; case SelectionClear: winDebug("SelectionClear - doing nothing\n"); From c03f9e23c2b75af410b2fc76ca3f1aa9e979dcc4 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Tue, 24 Sep 2013 15:09:22 +0100 Subject: [PATCH 5/9] hw/xwin: Add controls for enabling/disabling monitoring of PRIMARY selection xwinclip: Add -noprimary option Xwin: Add -primary and -noprimary options and tray-menu control v2: Use Bool type for fPrimarySelection Add -noprimary to usage message Fix indentation in hw/xwin/winwndproc.c Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/InitOutput.c | 4 ++++ hw/xwin/XWin.rc | 1 + hw/xwin/man/XWin.man | 6 +++++- hw/xwin/winclipboard/winclipboard.h | 2 ++ hw/xwin/winclipboard/xevents.c | 7 +++++-- hw/xwin/winclipboard/xwinclip.c | 7 +++++++ hw/xwin/winclipboard/xwinclip.man | 3 +++ hw/xwin/winprocarg.c | 24 ++++++++++++++++++++++++ hw/xwin/winresource.h | 1 + hw/xwin/wintrayicon.c | 19 +++++++++++++++++++ hw/xwin/winwndproc.c | 6 ++++++ 11 files changed, 77 insertions(+), 3 deletions(-) diff --git a/hw/xwin/InitOutput.c b/hw/xwin/InitOutput.c index eb19dceca..6d3f5a571 100644 --- a/hw/xwin/InitOutput.c +++ b/hw/xwin/InitOutput.c @@ -793,6 +793,10 @@ winUseMsg(void) #ifdef XWIN_CLIPBOARD ErrorF("-nounicodeclipboard\n" "\tDo not use Unicode clipboard even if on a NT-based platform.\n"); + + ErrorF("-[no]primary\n" + "\tWhen clipboard integration is enabled, map the X11 PRIMARY selection\n" + "\tto the Windows clipboard. Default is enabled.\n"); #endif ErrorF("-refresh rate_in_Hz\n" diff --git a/hw/xwin/XWin.rc b/hw/xwin/XWin.rc index a142f3070..a54e0fdbb 100644 --- a/hw/xwin/XWin.rc +++ b/hw/xwin/XWin.rc @@ -93,6 +93,7 @@ BEGIN POPUP "TRAYICON_MENU" BEGIN MENUITEM "&Hide Root Window", ID_APP_HIDE_ROOT + MENUITEM "Clipboard may use &PRIMARY selection", ID_APP_MONITOR_PRIMARY MENUITEM "&About...", ID_APP_ABOUT MENUITEM SEPARATOR MENUITEM "E&xit...", ID_APP_EXIT diff --git a/hw/xwin/man/XWin.man b/hw/xwin/man/XWin.man index a043ac281..15a57db02 100644 --- a/hw/xwin/man/XWin.man +++ b/hw/xwin/man/XWin.man @@ -174,7 +174,7 @@ on remote hosts, when that information is available and it's useful to do so. .SH OPTIONS CONTROLLING WINDOWS INTEGRATION .TP 8 .B \-[no]clipboard -Enables [disables] the integration between the Cygwin/X clipboard and +Enables [disables] the integration between the X11 clipboard and \fIWindows\fP clipboard. The default is enabled. .TP 8 .B "\-emulate3buttons [\fItimeout\fP]" @@ -200,6 +200,10 @@ prevents the \fIWindows\fP mouse cursor from being drawn on top of the X cursor. This parameter has no effect unless \fB-swcursor\fP is also specified. .TP 8 +.B \-[no]primary +Clipboard integration may [will not] use the PRIMARY selection. +The default is enabled. +.TP 8 .B \-swcursor Disable the usage of the \fIWindows\fP cursor and use the X11 software cursor instead. .TP 8 diff --git a/hw/xwin/winclipboard/winclipboard.h b/hw/xwin/winclipboard/winclipboard.h index 52481301b..9c5c568a7 100644 --- a/hw/xwin/winclipboard/winclipboard.h +++ b/hw/xwin/winclipboard/winclipboard.h @@ -33,4 +33,6 @@ void winFixClipboardChain(void); void winClipboardWindowDestroy(void); +extern Bool fPrimarySelection; + #endif diff --git a/hw/xwin/winclipboard/xevents.c b/hw/xwin/winclipboard/xevents.c index 3dca35496..daf11a18d 100644 --- a/hw/xwin/winclipboard/xevents.c +++ b/hw/xwin/winclipboard/xevents.c @@ -44,11 +44,13 @@ #endif #include -#include "internal.h" #include #include #include +#include "winclipboard.h" +#include "internal.h" + /* * Constants */ @@ -63,6 +65,7 @@ */ extern int xfixes_event_base; +Bool fPrimarySelection = TRUE; /* * Local variables @@ -793,7 +796,7 @@ winClipboardFlushXEvents(HWND hwnd, winDebug("winClipboardFlushXEvents - XFixesSetSelectionOwnerNotify\n"); /* Save selection owners for monitored selections, ignore other selections */ - if (e->selection == XA_PRIMARY) { + if ((e->selection == XA_PRIMARY) && fPrimarySelection) { MonitorSelection(e, CLIP_OWN_PRIMARY); } else if (e->selection == atomClipboard) { diff --git a/hw/xwin/winclipboard/xwinclip.c b/hw/xwin/winclipboard/xwinclip.c index 3677974c4..856c4dd54 100644 --- a/hw/xwin/winclipboard/xwinclip.c +++ b/hw/xwin/winclipboard/xwinclip.c @@ -92,6 +92,13 @@ main (int argc, char *argv[]) continue; } + /* Look for -noprimary */ + if (!strcmp (argv[i], "-noprimary")) + { + fPrimarySelection = False; + continue; + } + /* Yack when we find a parameter that we don't know about */ printf ("Unknown parameter: %s\nExiting.\n", argv[i]); exit (1); diff --git a/hw/xwin/winclipboard/xwinclip.man b/hw/xwin/winclipboard/xwinclip.man index 822db91d4..a53dc3029 100644 --- a/hw/xwin/winclipboard/xwinclip.man +++ b/hw/xwin/winclipboard/xwinclip.man @@ -29,6 +29,9 @@ Specifies the X server display to connect to. .TP 8 .B \-nounicodeclipboard Do not use unicode text on the clipboard. +.TP 8 +.B \-noprimary +Do not monitor the PRIMARY selection. .SH "SEE ALSO" XWin(1) diff --git a/hw/xwin/winprocarg.c b/hw/xwin/winprocarg.c index f6d14a1b8..e8cccb4c2 100644 --- a/hw/xwin/winprocarg.c +++ b/hw/xwin/winprocarg.c @@ -37,6 +37,10 @@ from The Open Group. #include "winmsg.h" #include "winmonitors.h" +#ifdef XWIN_CLIPBOARD +#include "winclipboard/winclipboard.h" +#endif + /* * Function prototypes */ @@ -707,6 +711,26 @@ ddxProcessArgument(int argc, char *argv[], int i) /* Indicate that we have processed this argument */ return 1; } + + /* + * Look for the '-primary' argument + */ + if (IS_OPTION("-primary")) { + fPrimarySelection = TRUE; + + /* Indicate that we have processed this argument */ + return 1; + } + + /* + * Look for the '-noprimary' argument + */ + if (IS_OPTION("-noprimary")) { + fPrimarySelection = FALSE; + + /* Indicate that we have processed this argument */ + return 1; + } #endif /* diff --git a/hw/xwin/winresource.h b/hw/xwin/winresource.h index afbf9f28d..37e92ce61 100644 --- a/hw/xwin/winresource.h +++ b/hw/xwin/winresource.h @@ -43,6 +43,7 @@ #define ID_APP_HIDE_ROOT 201 #define ID_APP_ALWAYS_ON_TOP 202 #define ID_APP_ABOUT 203 +#define ID_APP_MONITOR_PRIMARY 204 #define ID_ABOUT_WEBSITE 303 diff --git a/hw/xwin/wintrayicon.c b/hw/xwin/wintrayicon.c index e0aa7e5ab..6acc0d712 100644 --- a/hw/xwin/wintrayicon.c +++ b/hw/xwin/wintrayicon.c @@ -32,9 +32,13 @@ #ifdef HAVE_XWIN_CONFIG_H #include #endif + #include "win.h" #include #include "winprefs.h" +#ifdef XWIN_CLIPBOARD +#include "winclipboard/winclipboard.h" +#endif /* * Initialize the tray icon @@ -170,6 +174,21 @@ winHandleIconMessage(HWND hwnd, UINT message, RemoveMenu(hmenuTray, ID_APP_HIDE_ROOT, MF_BYCOMMAND); } +#ifdef XWIN_CLIPBOARD + if (g_fClipboard) { + /* Set menu state to indicate if 'Monitor Primary' is enabled or not */ + MENUITEMINFO mii = { 0 }; + mii.cbSize = sizeof(MENUITEMINFO); + mii.fMask = MIIM_STATE; + mii.fState = fPrimarySelection ? MFS_CHECKED : MFS_UNCHECKED; + SetMenuItemInfo(hmenuTray, ID_APP_MONITOR_PRIMARY, FALSE, &mii); + } + else { + /* Remove 'Monitor Primary' menu item */ + RemoveMenu(hmenuTray, ID_APP_MONITOR_PRIMARY, MF_BYCOMMAND); + } +#endif + SetupRootMenu(hmenuTray); /* diff --git a/hw/xwin/winwndproc.c b/hw/xwin/winwndproc.c index 76e487c0c..d4a953615 100644 --- a/hw/xwin/winwndproc.c +++ b/hw/xwin/winwndproc.c @@ -1218,6 +1218,12 @@ winWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) return 0; #endif +#ifdef XWIN_CLIPBOARD + case ID_APP_MONITOR_PRIMARY: + fPrimarySelection = !fPrimarySelection; + return 0; +#endif + case ID_APP_ABOUT: /* Display the About box */ winDisplayAboutDialog(s_pScreenPriv); From b4a08e642b977b4bbc892ff1d96ecc0cf6e2ca54 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Thu, 21 Nov 2013 15:18:48 +0000 Subject: [PATCH 6/9] hw/xwin: Improve reliability of clipboard X->Windows pastes Sometimes, particularly with large clipboard pastes to Windows, we could end up waiting for the timeout to expire, rather than pasting the data. Various changes to improve reliability: 1. Use XFlush() not XSync() in winProcessXEventsTimeout(). It makes no sense to ensure we have received replies to outstanding requests if we are going to wait for them using select() 2. Add XFlush() to winClipboardProc() Make sure we have sent any requests before we wait using select() 3. Don't use FD_ISSET() to check which fd is ready This looks like a Cygwin select() bug in that it sometimes returns 0 with an empty fd set before the timeout expires, but a fd appears to be ready. Add select() return value to debug output when we are warning that this has happened. 4. Drain event queues before entering select() Unconditionally drain event queues before entering select(). This seems to be the recommended way of writing select() and X event processing loops. winClipboardFlushXEvents() checks using XPending(), and winClipboardFlushWindowsMessageQueue() checks using PeekMessage() so this is safe against blocking, but means that may not need to enter select() at all sometimes. Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/thread.c | 50 ++++++++++++++++++---------------- hw/xwin/winclipboard/wndproc.c | 34 ++++++++++------------- 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/hw/xwin/winclipboard/thread.c b/hw/xwin/winclipboard/thread.c index be053574d..786b88972 100644 --- a/hw/xwin/winclipboard/thread.c +++ b/hw/xwin/winclipboard/thread.c @@ -255,22 +255,25 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) } } - /* Pre-flush X events */ - /* - * NOTE: Apparently you'll freeze if you don't do this, - * because there may be events in local data structures - * already. - */ data.fUseUnicode = fUseUnicode; - winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms); - /* Pre-flush Windows messages */ - if (!winClipboardFlushWindowsMessageQueue(hwnd)) { - ErrorF("winClipboardProc - winClipboardFlushWindowsMessageQueue failed\n"); - } - - /* Loop for X events */ + /* Loop for events */ while (1) { + + /* Process X events */ + winClipboardFlushXEvents(hwnd, + iWindow, pDisplay, &data, &atoms); + + /* Process Windows messages */ + if (!winClipboardFlushWindowsMessageQueue(hwnd)) { + ErrorF("winClipboardProc - winClipboardFlushWindowsMessageQueue trapped " + "WM_QUIT message, exiting main loop.\n"); + break; + } + + /* We need to ensure that all pending requests are sent */ + XFlush(pDisplay); + /* Setup the file descriptor set */ /* * NOTE: You have to do this before every call to select @@ -317,10 +320,9 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) break; } - /* Branch on which descriptor became active */ if (FD_ISSET(iConnectionNumber, &fdsRead)) { - /* Process X events */ - winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms); + winDebug + ("winClipboardProc - X connection ready, pumping X event queue\n"); } #ifdef HAS_DEVWINDOWS @@ -330,14 +332,16 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) if (1) #endif { - /* Process Windows messages */ - if (!winClipboardFlushWindowsMessageQueue(hwnd)) { - ErrorF("winClipboardProc - " - "winClipboardFlushWindowsMessageQueue trapped " - "WM_QUIT message, exiting main loop.\n"); - break; - } + winDebug + ("winClipboardProc - /dev/windows ready, pumping Windows message queue\n"); } + +#ifdef HAS_DEVWINDOWS + if (!(FD_ISSET(iConnectionNumber, &fdsRead)) && + !(FD_ISSET(fdMessageQueue, &fdsRead))) { + winDebug("winClipboardProc - Spurious wake, select() returned %d\n", iReturn); + } +#endif } winClipboardProc_Exit: diff --git a/hw/xwin/winclipboard/wndproc.c b/hw/xwin/winclipboard/wndproc.c index 23a0ac9bf..b63a1dc5c 100644 --- a/hw/xwin/winclipboard/wndproc.c +++ b/hw/xwin/winclipboard/wndproc.c @@ -83,8 +83,18 @@ winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay, fd_set fdsRead; long remainingTime; - /* We need to ensure that all pending events are processed */ - XSync(pDisplay, FALSE); + /* Process X events */ + iReturn = winClipboardFlushXEvents(hwnd, iWindow, pDisplay, data, atoms); + + winDebug("winProcessXEventsTimeout () - winClipboardFlushXEvents returned %d\n", iReturn); + + if ((WIN_XEVENTS_NOTIFY_DATA == iReturn) || (WIN_XEVENTS_NOTIFY_TARGETS == iReturn) || (WIN_XEVENTS_FAILED == iReturn)) { + /* Bail out */ + return iReturn; + } + + /* We need to ensure that all pending requests are sent */ + XFlush(pDisplay); /* Setup the file descriptor set */ FD_ZERO(&fdsRead); @@ -113,24 +123,8 @@ winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay, break; } - /* Branch on which descriptor became active */ - if (FD_ISSET(iConnNumber, &fdsRead)) { - /* Process X events */ - /* Exit when we see that server is shutting down */ - iReturn = winClipboardFlushXEvents(hwnd, - iWindow, pDisplay, data, atoms); - - winDebug - ("winProcessXEventsTimeout () - winClipboardFlushXEvents returned %d\n", - iReturn); - - if ((WIN_XEVENTS_NOTIFY_DATA == iReturn) || (WIN_XEVENTS_NOTIFY_TARGETS == iReturn) || (WIN_XEVENTS_FAILED == iReturn)) { - /* Bail out */ - return iReturn; - } - } - else { - winDebug("winProcessXEventsTimeout - Spurious wake\n"); + if (!FD_ISSET(iConnNumber, &fdsRead)) { + winDebug("winProcessXEventsTimeout - Spurious wake, select() returned %d\n", iReturn); } } From 94d433c8cb64c9167050d02473176f888decf1d8 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Fri, 21 Feb 2014 23:20:00 +0000 Subject: [PATCH 7/9] hw/xwin: Fix clipboard thread restart It seems that the clipboard thread restart mechanism has been broken for a while, which can be demonstrated using XDMCP with KDM (e.g. to a Kubutunu 12.04 host) KDM kills all attached clients, including the clipboard integration client, which restarts, but then exits on WM_QUIT. Using PostQuitMessage() in WM_DESTROY is unhelpful, as we may not actually be quitting the thread, if we just destroyed the window because the clipboard thread is about to retry, because he WM_QUIT message sticks around, and is noticed the next time we look at the window message queue and confuses us into thinking we need to quit. Sending a WM_DESTROY is apparently never correct anyhow, see [1] So: 1/ Use DestroyWindow() to destroy the clipboard messaging window when cleaning up for retry or exit in winClipboardProc (the clipboard thread main proc) 2/ Send a special WM_WM_QUIT message in winClipboardWindowDestroy() from the X server thread when the X server is resetting. 3/ When processing that WM_WM_QUIT message in the clipboard thread, cause the clipboard window to PostQuitMessage(), which causes the clipboard thread to exit. [1] http://blogs.msdn.com/b/oldnewthing/archive/2011/09/26/10216420.aspx Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/internal.h | 1 + hw/xwin/winclipboard/thread.c | 4 ++-- hw/xwin/winclipboard/wndproc.c | 6 ++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hw/xwin/winclipboard/internal.h b/hw/xwin/winclipboard/internal.h index bcf45ca4d..c6bde84af 100644 --- a/hw/xwin/winclipboard/internal.h +++ b/hw/xwin/winclipboard/internal.h @@ -44,6 +44,7 @@ #define WIN_XEVENTS_NOTIFY_TARGETS 4 #define WM_WM_REINIT (WM_USER + 1) +#define WM_WM_QUIT (WM_USER + 2) /* * References to external symbols diff --git a/hw/xwin/winclipboard/thread.c b/hw/xwin/winclipboard/thread.c index 786b88972..50e1e8cb5 100644 --- a/hw/xwin/winclipboard/thread.c +++ b/hw/xwin/winclipboard/thread.c @@ -351,7 +351,7 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) winClipboardProc_Done: /* Close our Windows window */ if (g_hwndClipboard) { - winClipboardWindowDestroy(); + DestroyWindow(g_hwndClipboard); } /* Close our X window */ @@ -491,7 +491,7 @@ void winClipboardWindowDestroy(void) { if (g_hwndClipboard) { - SendMessage(g_hwndClipboard, WM_DESTROY, 0, 0); + SendMessage(g_hwndClipboard, WM_WM_QUIT, 0, 0); } } diff --git a/hw/xwin/winclipboard/wndproc.c b/hw/xwin/winclipboard/wndproc.c index b63a1dc5c..a17f8a900 100644 --- a/hw/xwin/winclipboard/wndproc.c +++ b/hw/xwin/winclipboard/wndproc.c @@ -154,6 +154,12 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) ChangeClipboardChain(hwnd, s_hwndNextViewer); s_hwndNextViewer = NULL; + } + return 0; + + case WM_WM_QUIT: + { + winDebug("winClipboardWindowProc - WM_WM_QUIT\n"); PostQuitMessage(0); } From d172cd630dae9c991b84b4c367a4caf8199266ac Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Mon, 28 Apr 2014 12:48:15 +0100 Subject: [PATCH 8/9] hw/xwin: Fix hang on shutdown when we own the clipboard. If we are the clipboard owner when we are shutdown, we recieve a WM_RENDERALLFORMATS, to render the clipboard, so it's contents will remain available to other applications. Unfortunately, this is far too late to do anything useful with, as the server is waiting for the clipboard thread to exit, and so can't process requests to convert clipboard contents. Change so we just do nothing on WM_RENDERALLFORMATS. (I'm not convinced that WM_RENDERALLFORMATS has ever worked usefully, in any case). (To make this work, I guess we would need to rearrange the way shutdown works completely: first synchronously stop the clipboard, then stop the X server) We also then receive a WM_DRAWCLIPBOARD, perhaps telling us that the available clipboard formats have changed (as ones which haven't been rendered are now removed), but the clipboard owner is now the system, not us, which we have to arrange to ignore. Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/wndproc.c | 66 +++++++++++++--------------------- 1 file changed, 25 insertions(+), 41 deletions(-) diff --git a/hw/xwin/winclipboard/wndproc.c b/hw/xwin/winclipboard/wndproc.c index a17f8a900..1ea5bc6b7 100644 --- a/hw/xwin/winclipboard/wndproc.c +++ b/hw/xwin/winclipboard/wndproc.c @@ -143,6 +143,7 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) static Display *pDisplay; static Window iWindow; static ClipboardAtoms *atoms; + static Bool fRunning; /* Branch on message type */ switch (message) { @@ -160,7 +161,7 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) case WM_WM_QUIT: { winDebug("winClipboardWindowProc - WM_WM_QUIT\n"); - + fRunning = FALSE; PostQuitMessage(0); } return 0; @@ -176,6 +177,7 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) pDisplay = cwcp->pClipboardDisplay; iWindow = cwcp->iClipboardWindow; atoms = cwcp->atoms; + fRunning = TRUE; first = GetClipboardViewer(); /* Get handle to first viewer in chain. */ if (first == hwnd) @@ -308,6 +310,10 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) return 0; } + /* Bail when shutting down */ + if (!fRunning) + return 0; + /* * Do not take ownership of the X11 selections when something * other than CF_TEXT or CF_UNICODETEXT has been copied @@ -411,8 +417,21 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) winDebug("winClipboardWindowProc - WM_DESTROYCLIPBOARD - Ignored.\n"); return 0; - case WM_RENDERFORMAT: case WM_RENDERALLFORMATS: + winDebug("winClipboardWindowProc - WM_RENDERALLFORMATS - Hello.\n"); + + /* + WM_RENDERALLFORMATS is sent as we are shutting down, to render the + clipboard so it's contents remains available to other applications. + + Unfortunately, this can't work without major changes. The server is + already waiting for us to stop, so we can't ask for the rendering of + clipboard text now. + */ + + return 0; + + case WM_RENDERFORMAT: { int iReturn; Bool fConvertToUnicode; @@ -421,13 +440,11 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) ClipboardConversionData data; int best_target = 0; - winDebug("winClipboardWindowProc - WM_RENDER*FORMAT - Hello.\n"); + winDebug("winClipboardWindowProc - WM_RENDERFORMAT %d - Hello.\n", + wParam); /* Flag whether to convert to Unicode or not */ - if (message == WM_RENDERALLFORMATS) - fConvertToUnicode = FALSE; - else - fConvertToUnicode = (CF_UNICODETEXT == wParam); + fConvertToUnicode = (CF_UNICODETEXT == wParam); selection = winClipboardGetLastOwnedSelectionAtom(atoms); if (selection == None) { @@ -511,28 +528,6 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) atoms->atomLocalProperty, iWindow, CurrentTime); - /* Special handling for WM_RENDERALLFORMATS */ - if (message == WM_RENDERALLFORMATS) { - /* We must open and empty the clipboard */ - - /* Close clipboard if we have it open already */ - if (GetOpenClipboardWindow() == hwnd) { - CloseClipboard(); - } - - if (!OpenClipboard(hwnd)) { - ErrorF("winClipboardWindowProc - WM_RENDER*FORMATS - " - "OpenClipboard () failed: %08x\n", - GetLastError()); - } - - if (!EmptyClipboard()) { - ErrorF("winClipboardWindowProc - WM_RENDER*FORMATS - " - "EmptyClipboard () failed: %08x\n", - GetLastError()); - } - } - /* Process X events */ iReturn = winProcessXEventsTimeout(hwnd, iWindow, @@ -566,18 +561,7 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) SetClipboardData(CF_TEXT, NULL); } - /* Special handling for WM_RENDERALLFORMATS */ - if (message == WM_RENDERALLFORMATS) { - /* We must close the clipboard */ - - if (!CloseClipboard()) { - ErrorF("winClipboardWindowProc - WM_RENDERALLFORMATS - " - "CloseClipboard () failed: %08x\n", - GetLastError()); - } - } - - winDebug("winClipboardWindowProc - WM_RENDER*FORMAT - Returning.\n"); + winDebug("winClipboardWindowProc - WM_RENDERFORMAT - Returning.\n"); return 0; } } From 5920433c3a30f5f1c0ba1ab39a0c2ff388df6b23 Mon Sep 17 00:00:00 2001 From: Colin Harrison Date: Sat, 27 Sep 2014 11:50:11 +0100 Subject: [PATCH 9/9] hw/xwin: Don't allocate one wchar_t too much for unicode text placed on the Windows clipboard The count of wchar_t returned by MultiByteToWideChar() includes the terminating null character, so don't add one to it. Also, reduce the scope of various length variables Signed-off-by: Colin Harrison Reviewed-by: Jon TURNEY --- hw/xwin/winclipboard/xevents.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/hw/xwin/winclipboard/xevents.c b/hw/xwin/winclipboard/xevents.c index daf11a18d..835195b52 100644 --- a/hw/xwin/winclipboard/xevents.c +++ b/hw/xwin/winclipboard/xevents.c @@ -44,6 +44,7 @@ #endif #include +#include #include #include #include @@ -208,14 +209,11 @@ winClipboardFlushXEvents(HWND hwnd, int iReturn; HGLOBAL hGlobal = NULL; XICCEncodingStyle xiccesStyle; - int iConvertDataLen = 0; char *pszConvertData = NULL; char *pszTextList[2] = { NULL }; int iCount; char **ppszTextList = NULL; wchar_t *pwszUnicodeStr = NULL; - int iUnicodeLen = 0; - int iReturnDataLen = 0; Bool fAbort = FALSE; Bool fCloseClipboard = FALSE; Bool fSetClipboardData = TRUE; @@ -381,7 +379,7 @@ winClipboardFlushXEvents(HWND hwnd, /* Convert the Unicode string to UTF8 (MBCS) */ if (data->fUseUnicode) { - iConvertDataLen = WideCharToMultiByte(CP_UTF8, + int iConvertDataLen = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR) pszGlobalData, -1, NULL, 0, NULL, NULL); @@ -396,7 +394,6 @@ winClipboardFlushXEvents(HWND hwnd, } else { pszConvertData = strdup(pszGlobalData); - iConvertDataLen = strlen(pszConvertData) + 1; } /* Convert DOS string to UNIX string */ @@ -541,7 +538,6 @@ winClipboardFlushXEvents(HWND hwnd, */ case SelectionNotify: - winDebug("winClipboardFlushXEvents - SelectionNotify\n"); { char *pszAtomName; @@ -620,8 +616,7 @@ winClipboardFlushXEvents(HWND hwnd, /* Conversion succeeded or some unconvertible characters */ if (ppszTextList != NULL) { int i; - - iReturnDataLen = 0; + int iReturnDataLen = 0; for (i = 0; i < iCount; i++) { iReturnDataLen += strlen(ppszTextList[i]); } @@ -672,12 +667,12 @@ winClipboardFlushXEvents(HWND hwnd, if (data->fUseUnicode) { /* Find out how much space needed to convert MBCS to Unicode */ - iUnicodeLen = MultiByteToWideChar(CP_UTF8, + int iUnicodeLen = MultiByteToWideChar(CP_UTF8, 0, pszReturnData, -1, NULL, 0); - /* Allocate memory for the Unicode string */ - pwszUnicodeStr = malloc(sizeof(wchar_t) * (iUnicodeLen + 1)); + /* NOTE: iUnicodeLen includes space for null terminator */ + pwszUnicodeStr = malloc(sizeof(wchar_t) * iUnicodeLen); if (!pwszUnicodeStr) { ErrorF("winClipboardFlushXEvents - SelectionNotify " "malloc failed for pwszUnicodeStr, aborting.\n"); @@ -695,9 +690,10 @@ winClipboardFlushXEvents(HWND hwnd, /* Allocate global memory for the X clipboard data */ hGlobal = GlobalAlloc(GMEM_MOVEABLE, - sizeof(wchar_t) * (iUnicodeLen + 1)); + sizeof(wchar_t) * iUnicodeLen); } else { + int iConvertDataLen = 0; pszConvertData = strdup(pszReturnData); iConvertDataLen = strlen(pszConvertData) + 1; @@ -730,8 +726,7 @@ winClipboardFlushXEvents(HWND hwnd, /* Copy the returned string into the global memory */ if (data->fUseUnicode) { - memcpy(pszGlobalData, - pwszUnicodeStr, sizeof(wchar_t) * (iUnicodeLen + 1)); + wcscpy((wchar_t *)pszGlobalData, pwszUnicodeStr); free(pwszUnicodeStr); pwszUnicodeStr = NULL; }