Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Cstruct.sub_copy #305

Merged
merged 2 commits into from
Nov 18, 2022
Merged

add Cstruct.sub_copy #305

merged 2 commits into from
Nov 18, 2022

Conversation

c-cube
Copy link
Contributor

@c-cube c-cube commented Nov 17, 2022

I was looking for a way of copying a Cstruct.t without sharing the buffer (because the buffer is used for the next IO read), and did not find one.

Copy link
Member

@dinosaure dinosaure left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, you should probably extend Cstruct_cap too (even if nobody uses it...).

@c-cube
Copy link
Contributor Author

c-cube commented Nov 17, 2022

good point, done.

@dinosaure
Copy link
Member

Thanks, let's merge. I will try to find a window to cut a release.

@dinosaure dinosaure merged commit 63ca7cc into mirage:main Nov 18, 2022
@reynir
Copy link
Member

reynir commented Feb 9, 2023

Hi! I was just looking for this function in the interface without finding it. I was wondering if it would make sense to call it copy? I know there's already a copy, but it does the same as to_string (just without defaults) and I think it makes much more sense that copy returns a cstruct as well. There's something to figure out wrt. deprecation & migration. WDYT?

@c-cube c-cube deleted the cstruct-sub-copy branch February 9, 2023 19:24
@c-cube
Copy link
Contributor Author

c-cube commented Feb 9, 2023

I'm not a maintainer, but an option could be to put an alert on copy saying "this is just like to_string, were you looking for sub_copy?"?

@reynir reynir mentioned this pull request Feb 10, 2023
dinosaure added a commit to dinosaure/opam-repository that referenced this pull request Feb 20, 2023
…ct-lwt and cstruct-async (6.2.0)

CHANGES:

- Add `sub_copy` function (@c-cube, mirage/ocaml-cstruct#305)
- Fix documentation (@MisterDA, mirage/ocaml-cstruct#304)
- Add `to_hex_string` function (@c-cube, mirage/ocaml-cstruct#306)
- Fix documentation and use `Cstruct.length` instead of `Cstruct.len` (@reynir, mirage/ocaml-cstruct#307)
- Deprecate `copy` function (alias of `to_string`) (will be removed at the next minor release)
  (@reynir, mirage/ocaml-cstruct#308)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants