Add small signed integers #51
Conversation
| assert (check "-128" (to_string (of_int 128))); (* FIXME: This value seems strange? *) | ||
| assert (check "-128" (to_string (of_int (-128)))); | ||
| assert (check "127" (to_string (of_int (-129)))); (* FIXME: This value seems strange? *) | ||
| assert (check "-1" (to_string (of_int (-1)))); | ||
| assert (check "f" (to_hexstring (of_int (0xf)))); | ||
| assert (check "7f" (to_hexstring max_int)); | ||
| assert (check "80" (to_hexstring min_int)); | ||
| assert (check "81" (to_hexstring (of_int (-0x7f)))); (* FIXME: This value seems strange? *) |
There was a problem hiding this comment.
I found some of these results strange, but I guess this is more a result of of_int not doing any bounds checking? I can remove the FIXMEs if this is the expected behavior.
brendanzab
force-pushed
the
small-signed-integers-fixes
branch
from
9234869 to
a713eff
Compare
brendanzab
force-pushed
the
small-signed-integers-fixes
branch
from
feb654c to
d877b80
Compare
|
Just wondering if I could get approval to run the tests for this PR? |
|
Oh dear - there seems to be some buffer overflows on non-macOS platforms… I’m assuming from the original code? This might be a bit beyond me. 😬 |
|
It won’t be as fast, but in the interim I could just implement small integer parsing in terms of the existing let of_string s =
let x =
try int_of_string s with
| Failure _ -> failwith "Int8.of_string"
in
if x when x < min_value || x > max_value then
failwith "Int8.of_string"
else x |
|
The buffer overflows are due to a bug in the C code for printing the hex string representation of small integers (https://github.com/yallop/ocaml-integers/pull/51/files#diff-37aa1176c737be854df50dc3979f0cdfc6d74f3c060d2047cdf4bb4806bb6566R108-R114), which doesn't allocate enough space to hold the printed representation for negative numbers. |
This is an attempt to address some of the review comments on PR #49.
Closes #49