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

Conversion to Status seems wrong #24

Closed
ionut-arm opened this issue Jun 4, 2020 · 3 comments
Closed

Conversion to Status seems wrong #24

ionut-arm opened this issue Jun 4, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@ionut-arm
Copy link
Member

Currently the conversion From<psa_crypto_sys::psa_status_t> for Status converts any unsupported integer to a GenericError - I think that's not entirely correct as it aliases an error occuring in the crate with one relayed by the linked library.

The correct thing to do would be to have TryFrom and return a crate-specific error for anything outside of the recognised codes, and the correct enum variant for the correct ones. However I do get that TryFrom is not actually available without std so maybe we should have a method implemented straight on Status when no-std is on and the normal TryFrom when std is available.

@ionut-arm ionut-arm added the bug Something isn't working label Jun 4, 2020
@ionut-arm
Copy link
Member Author

I'm assuming that as the library gets more complex there'll be more need for crate-internal errors

@hug-dev
Copy link
Member

hug-dev commented Jun 10, 2020

I agree that it is not ideal but it is also a way to simplify error handling with less types to unwrap for the caller. It's nice that conceptually only PSA Crypto 1.0.0 abstracted types are used and not our own.

@hug-dev hug-dev closed this as completed Dec 11, 2020
@ionut-arm
Copy link
Member Author

Agreed to leave as is for now, since it's not a huge problem. If something new pops up in the future, we can just reopen this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants