Zondax / hello-rustee

Apache License 2.0
0 stars 2 forks source link

Improve error messages #1

Closed jleni closed 3 years ago

jleni commented 4 years ago

Reference: https://github.com/OP-TEE/optee_client/blob/57f1a7962509ce9de7a648ea04b6c470ead8fa82/public/tee_client_api.h#L167-L187

ghost commented 3 years ago

Hmm I'm a bit confused: in the hello-rustee the error codes are present under optee-common/src/tee_error.rs along with other errors not found in the documentation. Whilst I can see some are obviously added for the application (PairNotFound) they seem to be added outside the range allowed by the documentation (0x00000001 - 0xFFFEFFFF "Implementation defined")... Should we move the enum in zondee crate (since it's common for both REE and TA)? Then in the optee-common we can have our extension on those error codes:

//in optee-common
enum Error {
    Tee(zondee::TeeError),
    PairNotFound,
    ...
}

I'm asking because I don't think I understand why it had been done that way exactly so I'm not comfortable changing it

neithanmo commented 3 years ago

Those in the link are the client errors, but internally TA has other error codes as well, we can integrate them in the optee_common. the zondee crate doesn't have an Error trait implementation for the optee errors, just returns an error code. so let's use the optee-common.

ghost commented 3 years ago

the zondee crate doesn't have an Error trait implementation for the optee errors, just returns an error code.

Yeah that's what I'm suggesting changing: we can put error codes that are defined in the API inside that crate, since it would be shared between teec and utee, whilst we can put in optee-common our error codes... That way, zondee-* could be very easily reused for other TA applications whilst we keep our specific logic/errors elsewhere...

neithanmo commented 3 years ago

Sounds good. I do not remember if we may have a problem with zondee in ta, it uses serde but it is marketd as optional.. Just check this and if it goes well then great.