-
Notifications
You must be signed in to change notification settings - Fork 10
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 support for timeout on getting the next ssl record for a connection #327
Add support for timeout on getting the next ssl record for a connection #327
Conversation
6db3cc2
to
952b50c
Compare
3ce1375
to
dc88562
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a couple questions
micro-rdk/src/esp32/dtls.rs
Outdated
} | ||
Err(e) => { | ||
let _ = state.error.insert(e); | ||
if state.error.as_ref().unwrap().kind() == std::io::ErrorKind::WouldBlock { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match statement instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
log::debug!("intermetidate timer expired"); | ||
return 2; | ||
} | ||
if now > *self.fin.as_ref().unwrap() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we check fin before intermediate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch that was really wrong :)
@@ -426,24 +510,24 @@ impl SSLContext { | |||
} | |||
} | |||
|
|||
pub struct SSLStream<S> { | |||
pub(crate) struct SSLStream<S> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is holding a pointer and interacting with it in an unsafe way, is it ok that you don't have a drop implementation for de-allocating the memory behind the pointer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are talking about bio_ptr it should be dropped on line 519
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I missed that, my bad
9c57a47
to
44593cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.