use Bytes.t as data type for IPv6 addresses #115
Conversation
verbosemode
commented
Mar 9, 2023
verbosemode
commented
- this PR is a proposal to addresses issue Use 16-byte string for internal representation of IPv6 addresses #16
- I didn't touch any parsing functions, but migrated auxiliary functions to operate on Bytes. If this was over the top, I could cut this PR down to the essential parts and keep the rest for later.
- I've resorted to imperative code due to the index based set functions of Bytes
* this addresses issue mirage#16
|
The current build failures are mainly due to unused functions in the B128 module. But maybe it is a good idea to keep them for future PRs. |
|
I like the code changes, thanks a lot! :) I pushed a commit which removes some unused bindings -- if we need them at a later point, we can add them then :) -- this should make the CI green. Does this code fix #113 as a side-effect? Would be great to have it double-checked by @RyanGibb, and maybe have a test case? |
|
I can confirm that this fixed #113. Thanks @verbosemode ! |
|
@RyanGibb cool, great. Could you PR a test case for that issue you uncovered? |
Sorry for the noise, while at this PR, I pushed 5c4a09b as a test for your observed failure. This fails on the main branch, and succeeds on this branch. I'd move on to merge this PR (today or tomorrow) and cut a release, what do others think? @RyanGibb @avsm |
|
Apparently at the exact same minute as you commented I also opened a pr https://github.com/mirage/ocaml-ipaddr/pull/116/files 😄 I'm happy to go with your test though. The only thing is perhaps it would be better to use the address |
|
@RyanGibb I added the 0:0:800:: test case as well. |
…sexp and ipaddr-cstruct (5.4.0) CHANGES: * Use Bytes.t for IPv6 addresses (mirage/ocaml-ipaddr#115 @verbosemode, fixes mirage/ocaml-ipaddr#16 @dsheets) * Also fixes V6.to_int64 (reported by @RyanGibb in mirage/ocaml-ipaddr#113)
Thanks! |
