Skip to content

Refactor ssl module #3147

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

Merged
merged 1 commit into from
Sep 27, 2021
Merged

Refactor ssl module #3147

merged 1 commit into from
Sep 27, 2021

Conversation

youknowone
Copy link
Member

I moved io::Read and io::Write for PySocketRef to ssl. If they are implemented for PySocket, it would be member of socket. But the PyRef<PySocket> requirement is decided by ssl::SslStream. not by socket itself. So I moved it to ssl to make them specific to ssl requirement.

part of #3104

@@ -170,7 +168,7 @@ impl PySocket {
PyRwLockReadGuard::try_map(self.sock.read(), |sock| sock.get()).ok()
}

fn sock_io(&self) -> io::Result<PyMappedRwLockReadGuard<'_, Socket>> {
pub(crate) fn sock_io(&self) -> io::Result<PyMappedRwLockReadGuard<'_, Socket>> {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would make sense to expose a pub send(&[u8]) -> io::Result<usize> and recv(&mut [u8]) -> io::Result<usize> for once the stdlib gets moved out of rustpython-vm oh I guess socket is also in stdlib. But the above still seems like a better abstraction than exposing ReadGuard.

Copy link
Member

Choose a reason for hiding this comment

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

(and flush() for sockets is a no-op)

Copy link
Member Author

@youknowone youknowone Sep 27, 2021

Choose a reason for hiding this comment

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

thanks. I implemented io::Read and io::Write for &PySocket and used it from ssl

@youknowone youknowone merged commit 837bf59 into RustPython:main Sep 27, 2021
@youknowone youknowone deleted the ssl-module branch September 27, 2021 08:12
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