Suggested patch for 64-bit pbproxy in SL
Hi, After upgrading to SL, when using X11 I kept seeing "Atom list is not a multiple of the size of an atom!" in my system logs, and also noticed that copying between OSX and X11 was sometimes not working (especially when "Update Pasteboard immediately" was checked). The problem is with the following lines in x-selection.m: Atom a = None; if (pdata->length % sizeof (a)) { fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); return None; } On i386, sizeof(a)=4, but on x86_64, sizeof(a)=8. However, it looks like pdata->length is always a multiple of an int (4 bytes) even on x86_64, so about half the time this check gets flagged and the code to check the clipboard type is not run. My suggestion is just to use sizeof(int) instead of sizeof(a). Looking through the server code, it looks like Atoms are always 32-bit anyway when compiling the rest of the server, and are sizeof(long) outside of the server. Here's a copy of the patch I used to fix pbproxy: --- xorg-server-1.4.2-apple47/hw/xquartz/pbproxy/x-selection.m.orig +++ xorg-server-1.4.2-apple47/hw/xquartz/pbproxy/x-selection.m @@ -228,16 +228,16 @@ get_property(Window win, Atom property, TRACE (); - if (pdata->length % sizeof (a)) + if (pdata->length % sizeof (int)) { fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); return None; } - for (i = 0; i < pdata->length; i += sizeof (a)) + for (i = 0; i < pdata->length; i += sizeof (int)) { a = None; - memcpy (&a, pdata->data + i, sizeof (a)); + memcpy (&a, pdata->data + i, sizeof (int)); if (a == atoms->image_png) { @@ -440,10 +440,6 @@ get_property(Window win, Atom property, [self x_copy_request_targets]; } } - else - { - XBell (xpbproxy_dpy, 0); - } } /* Set pbproxy as owner of the SELECTION_MANAGER selection. With this patch, I was able to copy an image from the OSX clipboard to an X11 application (gnumeric), and later select this image in X11 and copy it back to OSX properly. I also removed a call to XBell in x-selection.m. For some reason, when I selected some text in an xterm (or other Xaw applications), whenever I clicked anywhere to un-highlight the selection the system would beep. This only seems to be happening in Xaw-based apps with "Update Pasteboard immediately" checked. The bell ringing was driving me crazy, so I removed the call to XBell which is probably not needed anyway. I hope this is useful, Martin
On Sep 4, 2009, at 07:41, Martin Otte wrote:
After upgrading to SL, when using X11 I kept seeing "Atom list is not a multiple of the size of an atom!" in my system logs, and also noticed that copying between OSX and X11 was sometimes not working (especially when "Update Pasteboard immediately" was checked).
I haven't looked at this in depth, but thanks for your analysis and patch. I'll get this checked into git after some testing. Can you please open a bug report as well?
On i386, sizeof(a)=4, but on x86_64, sizeof(a)=8. My suggestion is just to use sizeof(int) instead of sizeof(a). Looking through the server code, it looks like Atoms are always 32- bit anyway when compiling the rest of the server, and are sizeof (long) outside of the server.
With this patch, I was able to copy an image from the OSX clipboard to an X11 application (gnumeric), and later select this image in X11 and copy it back to OSX properly.
So that hints to the solution... but sizeof(a) should == sizeof(Atom) == sizeof(int) ... My guess is there's something up wrt this hunk in /usr/X11/include/X11/ Xdefs.h : #ifndef _XTYPEDEF_ATOM # define _XTYPEDEF_ATOM # ifndef _XSERVER64 typedef unsigned long Atom; # else typedef CARD32 Atom; # endif #endif I'm guessing _XSERVER64 isn't getting correctly #defined for 64bit somewhere...
I also removed a call to XBell in x-selection.m.
Thanks. I think that was old debugging that forgot to get punted.
For some reason, when I selected some text in an xterm (or other Xaw applications), whenever I clicked anywhere to un-highlight the selection the system would beep. This only seems to be happening in Xaw-based apps with "Update Pasteboard immediately" checked. The bell ringing was driving me crazy, so I removed the call to XBell which is probably not needed anyway.
I hope this is useful,
Yep, thanks.
Actually, from X.h: /* * _XSERVER64 must ONLY be defined when compiling X server sources on * systems where unsigned long is not 32 bits, must NOT be used in * client or library code. */ So it looks like the *CLIENT* Atom is supposed to be long. The *SERVER* Atom is supposed to be 32bits. ... Martin, can you #define TEST in x-selection.m and see what crops up in get_property() ... my guess is that the Atoms are being sent as 32bit from the server (format = 32bit)... and we just need to interpret them as 32bit values... but there should be a more elegant way in to do that... Maybe we'll just have to store format in the propdata struct... ick. this is sounding messy. Thanks, Jeremy On Sep 4, 2009, at 11:09, Jeremy Huddleston wrote:
On Sep 4, 2009, at 07:41, Martin Otte wrote:
After upgrading to SL, when using X11 I kept seeing "Atom list is not a multiple of the size of an atom!" in my system logs, and also noticed that copying between OSX and X11 was sometimes not working (especially when "Update Pasteboard immediately" was checked).
I haven't looked at this in depth, but thanks for your analysis and patch. I'll get this checked into git after some testing. Can you please open a bug report as well?
On i386, sizeof(a)=4, but on x86_64, sizeof(a)=8. My suggestion is just to use sizeof(int) instead of sizeof(a). Looking through the server code, it looks like Atoms are always 32- bit anyway when compiling the rest of the server, and are sizeof (long) outside of the server.
With this patch, I was able to copy an image from the OSX clipboard to an X11 application (gnumeric), and later select this image in X11 and copy it back to OSX properly.
So that hints to the solution... but sizeof(a) should == sizeof (Atom) == sizeof(int) ...
My guess is there's something up wrt this hunk in /usr/X11/include/ X11/Xdefs.h :
#ifndef _XTYPEDEF_ATOM # define _XTYPEDEF_ATOM # ifndef _XSERVER64 typedef unsigned long Atom; # else typedef CARD32 Atom; # endif #endif
I'm guessing _XSERVER64 isn't getting correctly #defined for 64bit somewhere...
I also removed a call to XBell in x-selection.m.
Thanks. I think that was old debugging that forgot to get punted.
For some reason, when I selected some text in an xterm (or other Xaw applications), whenever I clicked anywhere to un-highlight the selection the system would beep. This only seems to be happening in Xaw-based apps with "Update Pasteboard immediately" checked. The bell ringing was driving me crazy, so I removed the call to XBell which is probably not needed anyway.
I hope this is useful,
Yep, thanks.
_______________________________________________ Xquartz-dev mailing list Xquartz-dev@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/xquartz-dev
Hey Martin, could you give this patch a try? I think it should do the trick. diff --git a/hw/xquartz/pbproxy/x-selection.h b/hw/xquartz/pbproxy/x- selection.h index d5bfcb5..fc903d0 100644 --- a/hw/xquartz/pbproxy/x-selection.h +++ b/hw/xquartz/pbproxy/x-selection.h @@ -41,6 +41,7 @@ struct propdata { unsigned char *data; size_t length; + int format; }; struct atom_list { diff --git a/hw/xquartz/pbproxy/x-selection.m b/hw/xquartz/pbproxy/x- selection.m index cd540be..4f2d848 100644 --- a/hw/xquartz/pbproxy/x-selection.m +++ b/hw/xquartz/pbproxy/x-selection.m @@ -77,7 +77,7 @@ static struct { @implementation x_selection -static struct propdata null_propdata = {NULL, 0}; +static struct propdata null_propdata = {NULL, 0, 0}; #ifdef DEBUG static void @@ -212,6 +212,7 @@ get_property(Window win, Atom property, struct propdata *pdata, Bool delete, Ato pdata->data = buf; pdata->length = buflen; + pdata->format = format; return /*success*/ False; } @@ -223,21 +224,20 @@ get_property(Window win, Atom property, struct propdata *pdata, Bool delete, Ato - (Atom) find_preferred:(struct propdata *)pdata { Atom a = None; - size_t i; + size_t i, step; Bool png = False, jpeg = False, utf8 = False, string = False; TRACE (); - if (pdata->length % sizeof (a)) + if (pdata->format != 32) { - fprintf(stderr, "Atom list is not a multiple of the size of an atom! \n"); + fprintf(stderr, "Atom list is expected to be formatted as an array of 32bit values.\n"); return None; } - for (i = 0; i < pdata->length; i += sizeof (a)) + for (i = 0, step = pdata->format >> 3; i < pdata->length; i += step) { - a = None; - memcpy (&a, pdata->data + i, sizeof (a)); + a = (Atom)*(uint32_t *)(pdata->data + i); if (a == atoms->image_png) { On Sep 4, 2009, at 07:41, Martin Otte wrote:
Hi,
After upgrading to SL, when using X11 I kept seeing "Atom list is not a multiple of the size of an atom!" in my system logs, and also noticed that copying between OSX and X11 was sometimes not working (especially when "Update Pasteboard immediately" was checked).
The problem is with the following lines in x-selection.m: Atom a = None;
if (pdata->length % sizeof (a)) { fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); return None; }
On i386, sizeof(a)=4, but on x86_64, sizeof(a)=8. However, it looks like pdata->length is always a multiple of an int (4 bytes) even on x86_64, so about half the time this check gets flagged and the code to check the clipboard type is not run.
My suggestion is just to use sizeof(int) instead of sizeof(a). Looking through the server code, it looks like Atoms are always 32- bit anyway when compiling the rest of the server, and are sizeof(long) outside of the server.
Here's a copy of the patch I used to fix pbproxy:
--- xorg-server-1.4.2-apple47/hw/xquartz/pbproxy/x-selection.m.orig +++ xorg-server-1.4.2-apple47/hw/xquartz/pbproxy/x-selection.m @@ -228,16 +228,16 @@ get_property(Window win, Atom property,
TRACE ();
- if (pdata->length % sizeof (a)) + if (pdata->length % sizeof (int)) { fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); return None; }
- for (i = 0; i < pdata->length; i += sizeof (a)) + for (i = 0; i < pdata->length; i += sizeof (int)) { a = None; - memcpy (&a, pdata->data + i, sizeof (a)); + memcpy (&a, pdata->data + i, sizeof (int));
if (a == atoms->image_png) { @@ -440,10 +440,6 @@ get_property(Window win, Atom property, [self x_copy_request_targets]; } } - else - { - XBell (xpbproxy_dpy, 0); - } }
/* Set pbproxy as owner of the SELECTION_MANAGER selection.
With this patch, I was able to copy an image from the OSX clipboard to an X11 application (gnumeric), and later select this image in X11 and copy it back to OSX properly.
I also removed a call to XBell in x-selection.m. For some reason, when I selected some text in an xterm (or other Xaw applications), whenever I clicked anywhere to un-highlight the selection the system would beep. This only seems to be happening in Xaw-based apps with "Update Pasteboard immediately" checked. The bell ringing was driving me crazy, so I removed the call to XBell which is probably not needed anyway.
I hope this is useful, Martin _______________________________________________ Xquartz-dev mailing list Xquartz-dev@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/xquartz-dev
Hi, I tried this patch with version xorg-server-1.5.3-apple14, and copying works fine with no warnings in my console. I can copy graphics fine between OSX and X11 (using the gnumeric spreadsheat program) and back again, so the 64-bit pbproxy is working as well as the 32-bit version. Can you also remove the call to XBell around line 445 of x- selection.m? For some reason, the bell rings annoyingly whenever I click and un-highlight a selection in xterm when "Update Pasteboard immediately" is selected in the preferences. Thanks, Martin On Sep 5, 2009, at 4:40 PM, Jeremy Huddleston wrote:
Hey Martin, could you give this patch a try? I think it should do the trick.
diff --git a/hw/xquartz/pbproxy/x-selection.h b/hw/xquartz/pbproxy/x- selection.h index d5bfcb5..fc903d0 100644 --- a/hw/xquartz/pbproxy/x-selection.h +++ b/hw/xquartz/pbproxy/x-selection.h @@ -41,6 +41,7 @@ struct propdata { unsigned char *data; size_t length; + int format; };
struct atom_list { diff --git a/hw/xquartz/pbproxy/x-selection.m b/hw/xquartz/pbproxy/x- selection.m index cd540be..4f2d848 100644 --- a/hw/xquartz/pbproxy/x-selection.m +++ b/hw/xquartz/pbproxy/x-selection.m @@ -77,7 +77,7 @@ static struct {
@implementation x_selection
-static struct propdata null_propdata = {NULL, 0}; +static struct propdata null_propdata = {NULL, 0, 0};
#ifdef DEBUG static void @@ -212,6 +212,7 @@ get_property(Window win, Atom property, struct propdata *pdata, Bool delete, Ato
pdata->data = buf; pdata->length = buflen; + pdata->format = format;
return /*success*/ False; } @@ -223,21 +224,20 @@ get_property(Window win, Atom property, struct propdata *pdata, Bool delete, Ato - (Atom) find_preferred:(struct propdata *)pdata { Atom a = None; - size_t i; + size_t i, step; Bool png = False, jpeg = False, utf8 = False, string = False;
TRACE ();
- if (pdata->length % sizeof (a)) + if (pdata->format != 32) { - fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); + fprintf(stderr, "Atom list is expected to be formatted as an array of 32bit values.\n"); return None; }
- for (i = 0; i < pdata->length; i += sizeof (a)) + for (i = 0, step = pdata->format >> 3; i < pdata->length; i += step) { - a = None; - memcpy (&a, pdata->data + i, sizeof (a)); + a = (Atom)*(uint32_t *)(pdata->data + i);
if (a == atoms->image_png) {
On Sep 4, 2009, at 07:41, Martin Otte wrote:
Hi,
After upgrading to SL, when using X11 I kept seeing "Atom list is not a multiple of the size of an atom!" in my system logs, and also noticed that copying between OSX and X11 was sometimes not working (especially when "Update Pasteboard immediately" was checked).
The problem is with the following lines in x-selection.m: Atom a = None;
if (pdata->length % sizeof (a)) { fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); return None; }
On i386, sizeof(a)=4, but on x86_64, sizeof(a)=8. However, it looks like pdata->length is always a multiple of an int (4 bytes) even on x86_64, so about half the time this check gets flagged and the code to check the clipboard type is not run.
My suggestion is just to use sizeof(int) instead of sizeof(a). Looking through the server code, it looks like Atoms are always 32- bit anyway when compiling the rest of the server, and are sizeof (long) outside of the server.
Here's a copy of the patch I used to fix pbproxy:
--- xorg-server-1.4.2-apple47/hw/xquartz/pbproxy/x-selection.m.orig +++ xorg-server-1.4.2-apple47/hw/xquartz/pbproxy/x-selection.m @@ -228,16 +228,16 @@ get_property(Window win, Atom property,
TRACE ();
- if (pdata->length % sizeof (a)) + if (pdata->length % sizeof (int)) { fprintf(stderr, "Atom list is not a multiple of the size of an atom!\n"); return None; }
- for (i = 0; i < pdata->length; i += sizeof (a)) + for (i = 0; i < pdata->length; i += sizeof (int)) { a = None; - memcpy (&a, pdata->data + i, sizeof (a)); + memcpy (&a, pdata->data + i, sizeof (int));
if (a == atoms->image_png) { @@ -440,10 +440,6 @@ get_property(Window win, Atom property, [self x_copy_request_targets]; } } - else - { - XBell (xpbproxy_dpy, 0); - } }
/* Set pbproxy as owner of the SELECTION_MANAGER selection.
With this patch, I was able to copy an image from the OSX clipboard to an X11 application (gnumeric), and later select this image in X11 and copy it back to OSX properly.
I also removed a call to XBell in x-selection.m. For some reason, when I selected some text in an xterm (or other Xaw applications), whenever I clicked anywhere to un-highlight the selection the system would beep. This only seems to be happening in Xaw-based apps with "Update Pasteboard immediately" checked. The bell ringing was driving me crazy, so I removed the call to XBell which is probably not needed anyway.
I hope this is useful, Martin _______________________________________________ Xquartz-dev mailing list Xquartz-dev@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/xquartz-dev
participants (2)
-
Jeremy Huddleston
-
Martin Otte