mirage / charrua

A DHCP library in OCaml
http://mirage.github.io/charrua/
ISC License
55 stars 18 forks source link

Replace deprecated Cstruct.copy with to_string #123

Closed gridbugs closed 1 year ago

hannesm commented 1 year ago

Thanks for your PR, two minor nits, since off and len are optional keywords (and default to 0 / length - off), we can drop specifying them where they're the default.

I also prefer to have optional keywords used early in the function application (or wherever they are in the interface document).

hannesm commented 1 year ago

Thanks @gridbugs and @haesbaert, I updated this PR with your comments about len. Let's see whether CI passes.

haesbaert commented 1 year ago

Looks good to me ❤️. I'm traveling so please feel free to merge this.

On Mon, Apr 3, 2023, 17:27 Hannes Mehnert @.***> wrote:

@.**** commented on this pull request.

In lib/dhcp_wire.ml https://github.com/mirage/charrua/pull/123#discussion_r1156119702:

   in

let get_client_id () = if len < 2 then invalid_arg bad_len else

  • let s = Cstruct.to_string body ~off:1 ~len:(len - 1) in
  • let s = Cstruct.to_string ~off:1 body in

done in 732a8c3 https://github.com/mirage/charrua/pull/123/commits/732a8c3063bcea838bf0b29b5d2597be9c1a5ace

— Reply to this email directly, view it on GitHub https://github.com/mirage/charrua/pull/123#discussion_r1156119702, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABR2EALDM6CLCHNCG4HZH3W7LT4NANCNFSM6AAAAAAWOCSFGM . You are receiving this because you were mentioned.Message ID: @.***>

hannesm commented 1 year ago

Thanks a lot