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

wasm: add out_handle #16953

Merged
merged 1 commit into from
Mar 29, 2024
Merged

wasm: add out_handle #16953

merged 1 commit into from
Mar 29, 2024

Conversation

rockwotj
Copy link
Contributor

@rockwotj rockwotj commented Mar 7, 2024

As a nice way to interop with out ptrs introduce out_handle
similar to std::out_ptr_t but for handle.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x

Release Notes

  • none

As a nice way to interop with out ptrs introduce out_handle_adapter
similar to std::out_ptr_t but for handle.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Mar 8, 2024

@rockwotj rockwotj self-assigned this Mar 8, 2024
@rockwotj
Copy link
Contributor Author

rockwotj commented Mar 8, 2024

The plan here is to merge this, then I'll send a followup PR to pull it out into utils so that this can be used by the crypto/openssl work

@rockwotj rockwotj requested a review from oleiman March 28, 2024 15:42
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

Looks good. Just to check my understanding - general idea is that

  • out_handler is convertible to T**
  • some allocation/initialization of an object happens on the other side of ffi
  • pointer to initialized thing is assigned to T* _raw_ptr
  • ~out_handler() puts the T* under handler management
  • then the custom deleter is called during ~unique_ptr()

Do I have that right? Might benefit from a unit test under ffi_helpers or something if it's not a huge lift. Mostly provide an expressive example of ^^

@rockwotj
Copy link
Contributor Author

Do I have that right? Might benefit from a unit test under ffi_helpers or something if it's not a huge lift. Mostly provide an expressive example of ^^

Correct! It's pretty nifty. I am going to followup and move this into utils - I'll stick a test in there when this is pulled out.

@rockwotj rockwotj merged commit 3ba50c7 into redpanda-data:dev Mar 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda area/wasm WASM Data Transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants