janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.38k stars 217 forks source link

Fix buffer push uint max #1457

Closed pnelson closed 3 weeks ago

pnelson commented 3 weeks ago

I was using the newer buffer/push-uint[16|32|64] functions and ran up against issues with upper limits.

error: bad slot #2, expected 32 bit signed integer, got 3927367768
  in buffer/push-uint32 [src/core/buffer.c] on line 388

This fixes the issue. I think it is fair that negative numbers here raise bad slot errors so I modified those tests to test the upper bounds instead.

Well, buffer/push-uint16 anything 0xFFFF <= x <= 0xFFFFFFF will push "\xff\xff" without error until it exceeds uint32_t. We could add a similar janet_getuinteger16, janet_checkuint16 , and janet_checkuint16range if that's an issue. Might look better to rename the 32-bit functions for consistency as well in that case so I thought I'd get feedback first if this is enough or what path we should take.

bakpakin commented 3 weeks ago

The old behavior was certainly confusing. I also thing the behavior of pushing the uint16 integers is wrong, too - we should fix that.

bakpakin commented 3 weeks ago

That said, I should point out that the FFI module has a much more general implementation of pushing bytes to buffers.

For example,

(ffi/write :uint16 100 @"") # -> @"d\0"
(ffi/read :uint16 @"d\0") # -> 100
pnelson commented 3 weeks ago

Thanks for the quick turnaround! Control over endianness was required for my use case but I didn't see a way to do so with the FFI module.