Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

add basic support for async-io#41

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fpagliughi merged 3 commits intosocketcan-rs:masterfromjreppnow:async-io-support
May 10, 2023

Conversation

jreppnow
Copy link
Contributor

@jreppnowjreppnow commentedMay 6, 2023
edited
Loading

As discussed in#39, this adds basic support for async-io based async runtimes like async-std and smol. Couple of notes:

  • I did not add a direct constructor, instead relying on TryFrom<CanSocket/CanFdSocket>. This could be added easily though.
  • I did not add a method to get back to the synchronous socket, partially because I haven't figured out how to do that yet.
  • I pretty much only ported the read and write functionality, since setting socket options is not possible asynchronously on Linux. To maintain access to the different setup methods, I give access to the inner socket viaas_sync_socket methods.
  • Instead of using the mem::zeroed-based approach when reading, I actually went with the MaybeUninit strategy, which I believe to be sound. However, I can go back to the zeroed version used for the synchronous read operations. Now just using the synchronous methods.

Thanks for any feedback!

@fpagliughifpagliughi mentioned this pull requestMay 9, 2023
@jreppnow
Copy link
ContributorAuthor

I noticed, after looking at the corresponding tokio code for a little while, that I don't actually need to reimplement the read logic at all.. I'll push an update this evening.

fpagliughi reacted with thumbs up emoji

- use blocking read/write methods instead of custom implementation- use macro for entire implementation
@jreppnow
Copy link
ContributorAuthor

Rebased and cleaned up.

Copy link
Collaborator

@fpagliughifpagliughi left a comment

Choose a reason for hiding this comment

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

We'll probably want to work on this one and the tokio one to make sure that have the same (or nearly) the same API, but this is a good start in that direction.

@fpagliughifpagliughi merged commitf93c85a intosocketcan-rs:masterMay 10, 2023
@fpagliughi
Copy link
Collaborator

Oh, also, just FYI:

Instead of using the mem::zeroed-based approach when reading, I actually went with the MaybeUninit strategy, which I believe to be sound. However, I can go back to the zeroed version used for the synchronous read operations.

I realize you pulled this out from the PR, but in general, it's a standard practice in socket programming to zero out structs like socket addresses, an then just fill in the needed fields, knowing the rest are zero.

But you are right thatMaybeUninit would be fine for reading in frames, since a successful read is certain to fill the whole struct.

@jreppnow
Copy link
ContributorAuthor

jreppnow commentedMay 11, 2023
edited
Loading

@fpagliughi Yeah, certainly true when you are the one filling the data, but when passing a buffer or a frame as on out parameter to a kernel method for it to write into, there is generally no need to initialise (if you only use the the data if you don't get an error an only until the length returned by the kernel method). If you look at the examples in the kernel docs, you will see that they also do not initialise out buffers/frames:https://docs.kernel.org/networking/can.html

When filling my own struct in C (read: writing to a socket), I've personally moved away from memset(0) and just use always struct initialisation, as I consider it a lot safer (you can't forget about the memset):

structmy_struct {size_tsize,void*ptr, }voidmain(void) {// Stack versionstructmy_structms= {.size=14};// Non-mentioned values are "static default initialised", which means zeroed for primitivesassert(!ms.ptr);// through pointerstructmy_struct*ptr=malloc(sizeof(*ptr));*ptr= (structmy_struct) {.ptr=ptr};assert(!ptr->size);}
fpagliughi reacted with thumbs up emoji

@fpagliughi
Copy link
Collaborator

fpagliughi commentedMay 11, 2023
edited
Loading

BTW... I'm a little unsure of keeping the macro from this PR in the long run:

  • It's somewhat confusing to save just few lines of code.
  • It's considerably different than the way the other two socket families (tokio, blocking) are implemented, and I would like to remain consistent.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fpagliughifpagliughifpagliughi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@jreppnow@fpagliughi

[8]ページ先頭

©2009-2025 Movatter.jp