Re: [PATCH] Simplify the implementation of ios_mask functions.

[John Darrington]

On Thu, Dec 19, 2019 at 03:08:22PM +0100, Egeyar Bagcioglu wrote:

     > +  *byte &= ~0 >> 8 - significant_bits;
     
     The right-hand side of the statement above will create a number with "8 -
     significant_bits" many leading 0s followed by many 1s. Since "byte" has
     only its least significant byte set, this will leave it unchanged. What
     we want here instead is to and with significant_bits many 1s in the least
     significant bits, with as many leading 0s as necessary. I am being picky
     as this is a matter of correctness of the code, in spite of poke VM also
     fixing such errors.
     
     Can we use the following instead?
     
     *byte &= (1 << significant_bits) - 1;
     
     What do you think?


The problem with this, is that using the munus operator "-"  is only
going to work if the host platform uses two's complement arithmetic.
This is true for almost all systems today - but hey you said you're
being picky!   I think it is a mistake to mix bitwise operations with
arithmetic operations except in code which you're 100% sure is going
to run only one architecture.

But you're right about the ~0 - it's not going to limit itself to 8
bits.  So if that's what's needed an alternative would be:

  *byte &= 0xFFU >> 8 - significant_bits;

I hope this makes sense.

I think also, that both of these functions could use a comment, to make
it clearer exactly what they're supposed to be doing.


J'