[AC21.5R21.4] Patch: fix cut & past problem on 64 bit platforms
Stephen J. Turnbull
stephen at xemacs.org
Sat Feb 17 11:01:38 EST 2007
APPROVE COMMIT 21.5 RECOMMEND 21.4
Vin, I haven't actually checked, but the comments in these files smell
like they go back to maybe Jamie. ;-) 21.4 probably should get this
patch, too. It may not apply as is because it's got some of Ben's
newfangled naming in it.
Takashi Iwai writes:
> The last patch in the above is correct and doesn't lead to crashes.
OK.
> > (2) Now, this looks like a bug in the 64-bit X libraries to me. What
> > happens if it gets fixed?
>
> It's no implementation bug but a brain-dead API definition.
Oh, I think I can see why they did it. With any luck, your code will
work on 64-bit platforms running X11R4. (^^;
Here's the patch I'm putting into XEmacs. The bug report shows SuSE
has done extensive testing, and it clearly conforms to documentation.
I've verified that callers in XEmacs do indeed expect to receive
arrays of longs. The remaining question in my mind is whether this is
going to be right for older Xlibs on 64-bit platforms, but that's not
something that can be tested without access to implementations.
Mike & Takashi, I've taken the liberty of doing extensive revision to
the commentary, but the code is yours. I thought about fiddling with
some of the hard-coded assumptions, but really, we don't support
128-bit processors yet anyway, and it seems *really* unlikely that
there will ever again be a platform with anything but 8-bit chars and
16-bit shorts.
If you want any changes to the credits, let me know.
Index: src/ChangeLog
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/ChangeLog,v
retrieving revision 1.1044
diff -u -U0 -r1.1044 ChangeLog
--- src/ChangeLog 15 Feb 2007 16:12:13 -0000 1.1044
+++ src/ChangeLog 17 Feb 2007 15:45:05 -0000
@@ -0,0 +1,13 @@
+2007-02-18 Stephen J. Turnbull <stephen at xemacs.org>
+
+ Code by Mike FABIAN <mfabian at suse.de>, Takashi Iwai <tiwai at suse.de>.
+ See xemacs-beta <s3thctmf46c.fsf at magellan.suse.de>. Thanks!
+ Documentation marshalled by me.
+
+ * select-x.c (x_get_window_property):
+ The buffer for property data in 32-bit format is an array of longs,
+ which need not be 32-bit. Compute residual from partial reads and
+ buffer sizes correctly for sizeof(long) == 8.
+
+ * select-common.h: Refer to documentation in select-x.c.
+
Index: src/select-common.h
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/select-common.h,v
retrieving revision 1.6
diff -u -r1.6 select-common.h
--- src/select-common.h 25 Oct 2005 11:16:27 -0000 1.6
+++ src/select-common.h 17 Feb 2007 15:45:11 -0000
@@ -47,6 +47,10 @@
if > 16 bits: Cons of top16, bot16
* 32 > 1 Vector of the above
+ NOTE NOTE NOTE:
+ Format == 32 means that the buffer will be C longs, which need not be
+ 32-bit quantities. See the note in select-x.c (x_get_window_property).
+
When converting a Lisp number to C, it is assumed to be of format 16 if
it is an integer, and of format 32 if it is a cons of two integers.
Index: src/select-x.c
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/src/select-x.c,v
retrieving revision 1.23
diff -u -r1.23 select-x.c
--- src/select-x.c 2 Mar 2005 18:31:57 -0000 1.23
+++ src/select-x.c 17 Feb 2007 15:45:12 -0000
@@ -1048,7 +1048,42 @@
return;
}
- total_size = bytes_remaining + 1;
+ /* The manpage for XGetWindowProperty from X.org X11.7.2 sez:
+ nitems_return [[ our actual_size_ret ]]
+ Returns the actual number of 8-bit, 16-bit, or 32-bit items
+ stored in the prop_return data.
+ prop_return [[ our tmp_data ]]
+ Returns the data in the specified format. If the returned
+ format is 8, the returned data is represented as a char
+ array. If the returned format is 16, the returned data is
+ represented as a array of short int type and should be cast
+ to that type to obtain the elements. If the returned format
+ is 32, the property data will be stored as an array of longs
+ (which in a 64-bit application will be 64-bit values that are
+ padded in the upper 4 bytes).
+ bytes_after_return [[ our bytes_remaining ]]
+ Returns the number of bytes remaining to be read in the prop-
+ erty if a partial read was performed.
+
+ AFAIK XEmacs does not support any platforms where the char type is other
+ than 8 bits (Cray?), or where the short type is other than 16 bits.
+ There is no such agreement on the size of long, and 64-bit platforms
+ generally make long be a 64-bit quantity while while it's 32 bits on
+ 32-bit platforms.
+
+ This means that on all platforms the wire item is the same size as our
+ buffer unit when format == 8 or format == 16 or format == wordsize == 32,
+ and the buffer size can be taken as bytes_remaining plus padding.
+ However, when format == 32 and wordsize == 64, the buffer unit is twice
+ the size of the wire item. Obviously this code below is not 128-bit
+ safe. (We could replace the factor 2 with (sizeof(long)*8/32.)
+
+ We can hope it doesn't much matter on versions of X11 earlier than R7.
+ */
+ if (sizeof(long) == 8 && *actual_format_ret == 32)
+ total_size = 2 * bytes_remaining + 1;
+ else
+ total_size = bytes_remaining + 1;
*data_ret = xnew_rawbytes (total_size);
/* Now read, until we've gotten it all. */
@@ -1072,7 +1107,12 @@
reading it. Deal with that, I guess....
*/
if (result != Success) break;
- *actual_size_ret *= *actual_format_ret / 8;
+ /* Again we need to compute the number of bytes in our buffer, not
+ the number of bytes transferred for the property. */
+ if (sizeof(long) == 8 && *actual_format_ret == 32)
+ *actual_size_ret *= 8;
+ else
+ *actual_size_ret *= *actual_format_ret / 8;
memcpy ((*data_ret) + offset, tmp_data, *actual_size_ret);
offset += *actual_size_ret;
XFree ((char *) tmp_data);
More information about the XEmacs-Beta
mailing list
