[Xquartz-dev] Suggested patch for 64-bit pbproxy in SL

[Martin Otte]

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 at lists.macosforge.org
>> http://lists.macosforge.org/mailman/listinfo.cgi/xquartz-dev
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.macosforge.org/pipermail/xquartz-dev/attachments/20090914/ab0d7cfe/attachment.html>