The implementation of base_uint<BITS>::operator++(int) and base_uint<BITS>::operator--(int) is now safer. Array pn is accessed via index i after bounds checking has been performed on the index, rather than before.
The logic of the while loops has also been made more clear.
173 | @@ -174,7 +174,7 @@ class base_uint 174 | { 175 | // prefix operator 176 | int i = 0; 177 | - while (++pn[i] == 0 && i < WIDTH-1) 178 | + while (i < WIDTH && ++pn[i] == 0)
As I see it, this code can cause an out-of-bounds access only in the case of WIDTH=0, which would be pointless. Or am I missing something? I do agree that this change makes the code somewhat more readable.
Yes, the out-of-bounds access occurs in the edge case of WIDTH == 0. This template class can, although it probably won't, be instantiated with BITS < 32, something that would lead to WIDTH == 0. I was mainly interested in ensuring the out-of-bounds-check before array access regardless of such assumptions. Making it easier to follow the code logic is an extra gain.
This template class can, although it probably won't, be instantiated with BITS < 32
Indeed - as the code throughout the entire class is implemented, it can only be instantiated for a positive multiple of 32. Might make sense to add a compile-time assertion for that.
Do you think adding:
static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32.");
in the class constructor would be fine?
Yes, that'd be nice!
I have added the assertions, is there something more I can do for this PR?
The implementation of base_uint::operator++(int) and base_uint::operator--(int) is now safer.
Array pn is accessed via index i after bounds checking has been performed on the index, rather than before.
The logic of the while loops has also been made more clear.
A compile time assertion has been added in the class constructors to ensure that BITS is a positive multiple of 32.
Reopened with addition of static_assert for BITS value.
utACK