Don't depend on ppx_cstruct, add mli file#21
Don't depend on ppx_cstruct, add mli file#21reynir wants to merge 3 commits intohaesbaert:masterfrom
Conversation
The cstruct ppx was expanded and manually added in the source file. It was only used in one place for a relatively small struct. The hexdump function was omitted. A lib/rawlink_lowlevel.mli file was added with all members exposed. The intent is to eventually unexpose what is not needed.
They seem to be unused, and to me they seem at most useful for mock testing.
| external driver : unit -> driver = "caml_driver" | ||
| external unix_bytes_read : | ||
| Unix.file_descr -> Cstruct.buffer -> int -> int -> int | ||
| = "caml_unix_bytes_read" | ||
| external bpf_align : int -> int -> int = "caml_bpf_align" | ||
| val bpf_split_buffer : Cstruct.t -> int -> Cstruct.t list |
There was a problem hiding this comment.
I think we can remove bpf_align and bpf_split_buffer, and maybe driver as well from the interface.
| @@ -0,0 +1,13 @@ | |||
| type driver = AF_PACKET | BPF | |||
There was a problem hiding this comment.
I tried to not expose driver in the mli, but then the compiler warns that the constructors are never used to build values (warn 37). They are built in the C stubs. I guess we can unexpose it and silence the warning in the .ml file.
|
Heya, I'll look into this, just hang on tight, $LIFE has gotten me by the
balls.
…On Mon, Sep 9, 2024, 09:31 Reynir Björnsson ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/rawlink_lowlevel.mli
<#21 (comment)>:
> +external driver : unit -> driver = "caml_driver"
+external unix_bytes_read :
+ Unix.file_descr -> Cstruct.buffer -> int -> int -> int
+ = "caml_unix_bytes_read"
+external bpf_align : int -> int -> int = "caml_bpf_align"
+val bpf_split_buffer : Cstruct.t -> int -> Cstruct.t list
I think we can remove bpf_align and bpf_split_buffer, and maybe driver as
well from the interface.
------------------------------
In lib/rawlink_lowlevel.mli
<#21 (comment)>:
> @@ -0,0 +1,13 @@
+type driver = AF_PACKET | BPF
I tried to not expose driver in the mli, but then the compiler warns that
the constructors are never used to build values (warn 37). They are built
in the C stubs. I guess we can unexpose it and silence the warning in the
.ml file.
—
Reply to this email directly, view it on GitHub
<#21 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABR2EE365HU2EJD6KPH6Z3ZVVFDTAVCNFSM6AAAAABN34MZVWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOBZGA2DENJXGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Hey! No worries. After some thought this PR is not so important to me (my PR mirage/ocaml-cstruct#316 would alleviate this anyway). So feel free to close it or let it hang as you like. Please take care! |
If you ever feel overwhelmed, don't hesitate to reach out and we can take the repository to maintain (either mirage or robur-coop). Maybe adapt the goals and perspective of the repo (looking at the variety of IO systems supported, it may be sensible to figure which we actually want to support). |
3 participants
The cstruct ppx was expanded and manually added in the source file. It was only used in one place for a relatively small struct. The hexdump function was omitted.
A lib/rawlink_lowlevel.mli file was added with all members exposed. The intent is to eventually unexpose what is not needed.