2016-10-23 17:05 GMT+02:00 Dominik Vogt <dominik.v...@gmx.de>:
> On Sun, Oct 23, 2016 at 04:39:53PM +0200, Viktor Griph wrote:
>> Den 23 okt. 2016 14:36 skrev "Dominik Vogt" <dominik.v...@gmx.de>:
>> > void fev_sanitise_configure_request(XConfigureRequestEvent *cr)
>> > {
>> > if (cr->value_mask & CWX)
>> > {
>> > cr->x = (((int)cr->x) & 0xffff);
>> > }
>> > ...
>> > if (cr->value_mask & CWHeight)
>> > {
>> > cr->height = (((unsigned int)cr->height) &
>> 0xffff);
>> > }
>> > ...
>> > }
>> >
>> > This tries to deal with an inconsistency between the X protocol
>> > and the Xlib structures: In the X protocol, in the
>> > ConfirureWindow request, X and Y are signed "INT16" quantities and
>> > WIDTH and HEIGT are unsigned "CARD16" quantities, while in the
>> > Xlib structures they are "int" or "unsigned int".
>> >
>> > The code tries to cut off the excess bits from X and Y and does it
>> > wrong. With 32-bit integers, if cr->x is -1:
>> >
>> > ((int)-1) & 0xffff = 0xffffffff & 0xffff
>> > = 0xffff
>> > = 65535
>> >
>> > I'm not sure how to fix this. The easiest approach would be to
>> > cast these values to int16_t or uint16_t from stdint.h, but I
>> > vaguely remember there are some portability issues with the
>> > inttypes.s/stdint.h headers.
>>
>> Instead of setting the excess bits to 0 we need to set them to 1 for the
>> negative case.
>>
>> What about
>> if (cr->x & 0x8000)
>> {
>> cr->x = (((int)-1) ^ 0x7fff) | (((int)cr->x) & 0x7fff);
>> }
>> else
>> {
>> cr->x = (((int)cr->x) & 0xffff);
>> }
>>
>> The negative case could be simplified to
>>
>> (((int)-1) ^ 0x7fff) | ((int)cr->x)
>>
>> Or
>>
>> ((int)-1)<<16|((int)cr->x)
>
> I never can remember this; is it safe (in C) to assume that
> negative integers are stored in two-complement format? (Of course
> the old code makes the same assumtion, but it's broken and I want
> to fix it.)
So use (~(int)0) instead of -1 and it should work for both
one-complement and two-complement numbers. The MSB would still mark
the sign. (If what the code is trying to fix is that there may be
extra bits beyond the 16 bits that are defined in the X-server, then
those bits could be anything, and maybe wouldn't guarantee that
negative numbers are correctly 1-padded above bit 16, in fact they may
not even have the int MSB set. If that is the case then I don't think
casting to int16_t or uint16_t would be feasible. The standard
mandates the int16_t to be in two-complement form even if the hardware
is using one-complement so casting from int to int_16t on a
one-complement platform would change the bit pattern in order to
maintain the value, but if the value is out of range for the 16-bit
number due to the excess bits you are trying to correct. I'm not sure
if the handling of overflow at a cast to xxx16_t is well-defined cross
all platforms.
>
> If it's really portable, the cleanest way is casting to int16_t
> and uint16_t from stdint.h.
Definitely. I just don't know if it is.
