Skip to content

socketpool, ssl: rationalize return types & throwing exceptions #7638

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

Open
jepler opened this issue Feb 23, 2023 · 4 comments
Open

socketpool, ssl: rationalize return types & throwing exceptions #7638

jepler opened this issue Feb 23, 2023 · 4 comments
Milestone

Comments

@jepler
Copy link

jepler commented Feb 23, 2023

While looking into #7606 @dhalbert noted that there were irregularities in our socket-like APIs that may have contributed to the bug existing in the first place.

We should consider rationalizing the code so that:

  • the common-hal APIs consistently return either positive counts or negative errno values, in signed types (mp_int_t)
  • the shared-bindings have the responsibility of transforming negative errno values into OSError exceptions
  • noting, at least in comments, common-hal APIs that are not permitted to assume the VM is running (no alloc, no exception)
@jepler jepler added this to the 8.1.0 milestone Feb 23, 2023
@jepler jepler self-assigned this Feb 23, 2023
@tannewt
Copy link
Member

tannewt commented Feb 23, 2023

I (inconsistently) drop the common_hal_ prefix on functions meant to be used outside of the VM.

@gneverov
Copy link

Without the common_hal_ prefix the functions can sometimes collide with functions in 'shared-bindings' which already typically don't have a prefix.

@tannewt
Copy link
Member

tannewt commented Feb 23, 2023

Yup! I usually add a _ prefix which I use for statics ala Python's private designation.

@dhalbert
Copy link
Collaborator

Maybe we could use a base_ prefix or similar for the functions that can be called from C outside the VM. Maybe you can think of a better prefix. I too usually drop the common_hal_ for routines not meant to be called via a Python API.

@dhalbert dhalbert modified the milestones: 8.1.0, 8.x.x Apr 14, 2023
@jepler jepler modified the milestones: 8.x.x, Long term Jun 21, 2023
@jepler jepler removed their assignment Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants