[MPlayer-dev-eng] [PATCH] x11_common: XChangeProperty uses long for 32-bits

[Alexander Strasser]

Hello!

Reimar Döffinger wrote:
> On Mon, Aug 15, 2011 at 07:10:48PM +0200, Nicolas George wrote:
> > I found this patch while arguing with someone on whether OpenBSD's MPlayer
> > maintainer is reporting the patches upstream. It looks correct, although I
> > do not have access to a 64-bits big-endian box to check that it actually
> > changes something.
> 
> I think it really needs to be tested on a 64-bit big-endian system,
> because on such one only one of these can work.
> There are also questions about what should happen if pid_t is 64 bit
> on 32-bit system?
> In general what if the type is not compatible with long?
> Is it possible that the proposed change would not even compile.
> Should maybe a union {pid_t pid; long arg;} be used?
> And lastly: What is that code good for anyway??
> Because my opinion is that this is a really crappy and broken
> API and it would be preferable to just not support it and remove
> that code. If it's important maybe someone will make the effort
> of making a non-broken replacement.

  I have this thing on my list since forever. I am not sure for what
this is used by the window managers. I thought the kill-application-
with-the-unresponsive-window dialogs would use this information, but
it's just a guess. If that really is the case, then we should be
careful to only set correct values for that property or none at all.

  I really dislike the Xlib interface, and before anyone changes it
he should look up the documentation of the XChangeProperty function
and the relevant part of the EWMH spec that introduces this property.

XChangeProperty(3):
> ... (5th parameter)
> 
>   format    Specifies whether the data should be viewed as a list of
>             8-bit, 16-bit, or 32-bit quantities.  Possible values are
>             8,
>             16, and 32.  This information allows the X server to cor‐
>             rectly perform byte-swap operations as necessary.  If the
>             format is 16-bit or 32-bit, you must explicitly cast your
>             data pointer to an (unsigned char *) in the call to
>             XChange‐
>             Property.
> 
> ... (later in a description about XChangeProperty)
> 
>   If the specified format is 8, the property data must be a char
>   array.
>   If the specified format is 16, the property data must be a short
>   array.
>   If the specified format is 32, the property data must be a long
>   array.
> 
> ... (also interesting snippet of X_Get_Property description)
> 
>   N = actual length of the stored property in bytes
>                  (even if the format is 16 or 32)
>             I = 4 * long_offset
>             T = N - I
>             L = MINIMUM(T, 4 * long_length)
>             A = N - (I + L)
> 
>        The returned value starts at byte index I in the property
>        (index‐
>        ing from zero), and its length in bytes is L.  If the value for
>        long_offset causes L to be negative, a BadValue error results.
>        The value of bytes_after_return is A, giving the number of
>        trail‐
>        ing unread bytes in the stored property.
> 
>   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 short array and should be cast to that type to obtain the ele‐
>   ments.  If the returned format is 32, the returned data is
>   represented
>   as a long array and should be cast to that type to obtain
>   theelements.
> 
> ... (where: )
> 
>   long_length
>             Specifies the length in 32-bit multiples of the data to be
>             retrieved.
> 
>   long_offset
>             Specifies the offset in the specified property (in 32-bit
>             quantities) where the data is to be retrieved.
>

  I came up with the attached patch. But I did not yet find a way to
test it sufficiently. Comments and suggestions welcome!

  If the property is not that useful at all, I am also OK with not
setting it at all.

Greetings,
  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-x11_common-pid_transfer_v2.diff
Type: text/x-diff
Size: 771 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110815/9fa39cbe/attachment.bin>