[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
## Re: [PATCH] Simplify the implementation of ios_mask functions.

**From**: |
John Darrington |

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

**Date**: |
Thu, 19 Dec 2019 18:33:14 +0100 |

**User-agent**: |
NeoMutt/20170113 (1.7.2) |

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'