bytes2hexstring hook#1213
Conversation
|
@jberthold, is there a spreadsheet or similar for documenting hook implementations? |
- Added code comment - Renamed hook: bytes2hexstring -> bytes2hex - Improved test case - Use safe length to avoid potential out-of-bounds error
tothtamas28
left a comment
tothtamas28
left a comment
There was a problem hiding this comment.
LGTM. I'll leave the final approval to @Robertorosmaninho.
Co-authored-by: Tamás Tóth <tothtamas28@users.noreply.github.com>
Robertorosmaninho
left a comment
Robertorosmaninho
left a comment
There was a problem hiding this comment.
LGTM, just one question:
| parser_file="$(mktemp tmp.parse.XXXXXXXXXX)" | ||
| temp_inputs=() | ||
|
|
||
| # shellcheck disable=SC2329 |
There was a problem hiding this comment.
Sorry, why is this needed here?
There was a problem hiding this comment.
The shell check CI workflow kept failing, see for example this action: https://github.com/runtimeverification/llvm-backend/actions/runs/17319617915/job/49169763898
If I understand correctly, it's a false positive because the function is used in the following line. So I marked it to skip the check.
It's not related to this PR, I think, but maybe some CI flakiness caused by a shell check update.
There was a problem hiding this comment.
Got it! Thanks for clarifying it!
automergerpr-permission-manager
bot
merged commit fd42f24
into
develop
automergerpr-permission-manager
bot
deleted the
raoul/bytes2hexstring-dev
branch
3 participants
This PR adds a new hook implementation for converting byte arrays into hex strings.
This is needed by the kontrol-node to encode memory as JSON strings efficiently.