fix: safer integer conversion and external buffer support#6
fix: safer integer conversion and external buffer support#6wemeetagain merged 2 commits intomainfrom
Conversation
nazarhussain
changed the title
| const n: i64 = if (i.bits <= 32) | ||
| @as(i64, try value.getValueUint32()) | ||
| else | ||
| try value.getValueInt64(); |
There was a problem hiding this comment.
clarifying question: i know this was the same code previously, but how come value.getValueInt64() works here for >u32? does the napi api require an i64 result?
There was a problem hiding this comment.
napi doesn't provide a napi_get_value_uint64 — the only 64-bit integer API is napi_get_value_int64 which returns i64. So for unsigned types >32 bits we're forced to read as i64 first, then std.math.cast handles the range check (rejecting negative values or values that overflow the target type).
| return try env.createInt64(n); | ||
| } | ||
|
|
||
| const n = std.math.cast(u64, v) orelse return error.InvalidArg; |
There was a problem hiding this comment.
do we need to do this intermediate cast?
There was a problem hiding this comment.
Related to above feedback.
There was a problem hiding this comment.
still not sure what the problem with std.math.cast(.., v) is below in the if and else.
There was a problem hiding this comment.
Removed the intermediate cast. Now the values are casted at the return statements.
lodekeeper-z
left a comment
lodekeeper-z
left a comment
There was a problem hiding this comment.
LGTM — replacing @intCast with std.math.cast is the right call for NAPI boundaries where JS can pass arbitrary values. The external_buffer support is a nice addition too.
4 participants
std.math.castwith proper error handling instead of@intCast(prevents runtime panic on overflow)external_bufferreturn hint support