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

fix(ffi): add repr(transparent) to hyper_context #3191

Merged
merged 1 commit into from
Apr 3, 2023
Merged

fix(ffi): add repr(transparent) to hyper_context #3191

merged 1 commit into from
Apr 3, 2023

Conversation

LegionMammal978
Copy link
Contributor

The default representation does not guarantee the absence of initial padding, which is necessary for the transmute to be sound.

The default representation does not guarantee the absence of initial
padding, which is necessary for the transmute to be sound.
Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Thanks!

@seanmonstar seanmonstar merged commit 187b8f2 into hyperium:master Apr 3, 2023
@LegionMammal978 LegionMammal978 deleted the repr-transparent-context branch April 3, 2023 17:36
seanmonstar added a commit that referenced this pull request Apr 3, 2023
seanmonstar added a commit that referenced this pull request Apr 3, 2023
@seanmonstar
Copy link
Member

Sorry, I had to revert this in #3193, since I accidentally merged it before checking CI was happy. At the least, it's changed how the hyper.h file is generated. I can't yet tell if the change is important.

@LegionMammal978
Copy link
Contributor Author

Whoops! I hadn't noticed that this project uses cbindgen. Indeed, it doesn't seem to do what we want, referring to a nonexistent Context type:

@@ -158,11 +158,6 @@
 typedef struct hyper_clientconn_options hyper_clientconn_options;
 
 /*
- An async context for a task that contains the related waker.
- */
-typedef struct hyper_context hyper_context;
-
-/*
  A more detailed error object returned by some hyper functions.
  */
 typedef struct hyper_error hyper_error;
@@ -206,15 +201,20 @@
 
 typedef int (*hyper_body_foreach_callback)(void*, const struct hyper_buf*);
 
-typedef int (*hyper_body_data_callback)(void*, struct hyper_context*, struct hyper_buf**);
+/*
+ An async context for a task that contains the related waker.
+ */
+typedef Context hyper_context;
+
+typedef int (*hyper_body_data_callback)(void*, hyper_context*, struct hyper_buf**);
 
 typedef void (*hyper_request_on_informational_callback)(void*, struct hyper_response*);
 
 typedef int (*hyper_headers_foreach_callback)(void*, const uint8_t*, size_t, const uint8_t*, size_t);
 
-typedef size_t (*hyper_io_read_callback)(void*, struct hyper_context*, uint8_t*, size_t);
+typedef size_t (*hyper_io_read_callback)(void*, hyper_context*, uint8_t*, size_t);
 
-typedef size_t (*hyper_io_write_callback)(void*, struct hyper_context*, const uint8_t*, size_t);
+typedef size_t (*hyper_io_write_callback)(void*, hyper_context*, const uint8_t*, size_t);
 
 #ifdef __cplusplus
 extern "C" {
@@ -743,7 +743,7 @@
 /*
  Copies a waker out of the task context.
  */
-struct hyper_waker *hyper_context_waker(struct hyper_context *cx);
+struct hyper_waker *hyper_context_waker(hyper_context *cx);
 
 /*
  Free a waker that hasn't been woken.

And if we try replacing it with #[repr(C)], then it writes out the whole struct definition, again including the Context type. Looking at cbindgen, the only official way to get it to generate an opaque item is not to place any repr() attribute on the structure. Curiously, there's also an unofficial way, which is to do anything that would make cbindgen throw an error on the struct, e.g., prefix it with /// cbindgen:==. But this is a sticky issue all around, and it's hard for me to tell which solution would be best.

0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this pull request Jan 12, 2024
The default representation does not guarantee the absence of initial
padding, which is necessary for the transmute to be sound.
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this pull request Jan 12, 2024
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this pull request Jan 16, 2024
The default representation does not guarantee the absence of initial
padding, which is necessary for the transmute to be sound.

Signed-off-by: Sven Pfennig <[email protected]>
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this pull request Jan 16, 2024
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.

2 participants