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

Re-export header-only functions from liburing #7

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

withoutboats
Copy link
Collaborator

Closes #4. This will allow us in iou to delegate all of the prep APIs to liburing instead of being responsible for setting SQEs up correctly.

@twissel would you be open to doing a quick review of this code to check that I didn't get any argument or return types wrong?

@twissel
Copy link

twissel commented Dec 2, 2019

Two quick questions:

  1. Shouldn't addr https://github.com/withoutboats/uring-sys/blob/73cd794de0ccf9891098b7b3456d480ea5677ebc/src/lib.rs#L251 be declared as *mut libc::c_void? *const libc::c_void is good for everything except
    IORING_OP_READ_FIXED, and indeed it is declared as *mut libc::c_void here https://github.com/withoutboats/uring-sys/blob/73cd794de0ccf9891098b7b3456d480ea5677ebc/src/lib.rs#L269
  2. Passing libc::off_t as offset in prep_{read, write} functions, for example here https://github.com/withoutboats/uring-sys/blob/73cd794de0ccf9891098b7b3456d480ea5677ebc/src/lib.rs#L290 seems strange for me, because at least on my machine this is i64 and we have u64 here https://github.com/withoutboats/uring-sys/blob/73cd794de0ccf9891098b7b3456d480ea5677ebc/src/lib.rs#L64

@withoutboats
Copy link
Collaborator Author

withoutboats commented Dec 2, 2019

Agreed these are odd, but they're the types liburing exposes. I just checked: io_uring_prep_rw takes the addr as const void *addr and the offsets are passed as off_t.

@withoutboats
Copy link
Collaborator Author

And yea, everything but read_fixed also takes a const pointer, I assume because only read_fixed writes directly into that buffer.

@twissel
Copy link

twissel commented Dec 2, 2019

Btw: do you plan to use prep helpers in iou?

@withoutboats
Copy link
Collaborator Author

Yes, to let liburing handle the subtleties of preparing IO events properly.

Copy link

@twissel twissel left a comment

Choose a reason for hiding this comment

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

Everything looks good to me.

@withoutboats withoutboats merged commit 3df778f into master Dec 3, 2019
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.

Possibly add other header-only functions
2 participants