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

Alexander Strasser eclipse7 at gmx.net
Wed Aug 17 23:15:34 CEST 2011


Hi

Nicolas George wrote:
> Le decadi 30 thermidor, an CCXIX, Alexander Strasser a écrit :
> >   Would be good if someone could test, and verify if it works
> > as intended. I will also try to test it myself, but I can't
> > say when I will come to it.
> 
> I'll try to ask around if someone has a suitable box to test.
> 
> > +    if (prop != pid) { return; }
> 
> I don't think it can make any difference, but "(pid_t)prop != pid" may be
> more reliable than using the default integer promotions.

  OK. I took that in, seems reasonable.

> 
> >      XChangeProperty(display, window, XA_NET_WM_PID, XA_CARDINAL, 32,
> >                      PropModeReplace, (unsigned char *) &pid, 1);
> 
> I believe it should be &prop instead of &pid here.

  Of course! Good catch. It is always the simplest things, that can go
awfully wrong. Thank you for having a close look.

  Looking at my comment again, we are not checking if the PID fits into
32 bits, I guess this way it might happen that the X server receives a
partial PID. Could we use a bitwise AND operation to only assign bits
0 to 31, so that we do the early return when pid contains a value that
doesn't fit into 32 bits? That approach might fail on sign extension
of a negative value of a signed 32-bit pid_t into a signed 64 bit long
type in some cases. But I would like to think that is more hypothetical.

  Updated patch attached.

  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-x11_common-pid_transfer_v5.diff
Type: text/x-diff
Size: 763 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20110817/e81227ce/attachment.bin>


More information about the MPlayer-dev-eng mailing list