changeset 3833:2b1e7cb9ae2c

[xemacs-hg @ 2007-02-17 15:55:21 by stephent] Fix 64-bit issues in X property handling. <87d548dgqz.fsf@uwakimon.sk.tsukuba.ac.jp>
author stephent
date Sat, 17 Feb 2007 15:55:22 +0000
parents 21b3a0f9cbfd
children 6a549c70de54
files src/ChangeLog src/select-common.h src/select-x.c
diffstat 3 files changed, 59 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/src/ChangeLog	Fri Feb 16 22:51:16 2007 +0000
+++ b/src/ChangeLog	Sat Feb 17 15:55:22 2007 +0000
@@ -1,3 +1,16 @@
+2007-02-18  Stephen J. Turnbull  <stephen@xemacs.org>
+
+	Code by Mike FABIAN <mfabian@suse.de>, Takashi Iwai <tiwai@suse.de>.
+	See xemacs-beta <s3thctmf46c.fsf@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.
+
 2007-02-16  Stephen J. Turnbull  <stephen@xemacs.org>
 
 	* config.h.in: Move CANNA_NEW_WCHAR_AWARE here from canna_api.c.
--- a/src/select-common.h	Fri Feb 16 22:51:16 2007 +0000
+++ b/src/select-common.h	Sat Feb 17 15:55:22 2007 +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.
 
--- a/src/select-x.c	Fri Feb 16 22:51:16 2007 +0000
+++ b/src/select-x.c	Sat Feb 17 15:55:22 2007 +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);