OP-TEE / optee_docs

This git contains the official documentation for the OP-TEE project
BSD 2-Clause "Simplified" License
58 stars 96 forks source link

coding standard: defining unsigned integer constants using U() #117

Closed jenswi-linaro closed 3 years ago

jenswi-linaro commented 3 years ago
Misra C rule 7.2 says:
"A 'u' or 'U' suffix shall be applied to all integer constants that are
represented in an unsigned type"

Besides Misra this makes sense anyway to make the sign and size of the
integer well defined instead of depending on the size of the constant or
version of C standard a compiler is using.

Do this with the U() macro instead of an explicit 'U' at the end to make
the defines usable from assembly too.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

This is a step at trying to align more with Misra C recommendations. There's quite a few recommendations to contemplate, with some quite intrusive or perhaps even ill suited for OP-TEE. But this recommendation (rule 7.2) isn't too bad and a is a good start.

Given how many rules there are we can't list everyone here instead I propose that we expand the guidelines here as needed, with the intent to do so on their own merits. What we write here are the rules we follow. They just happen to also take care of Misra recommendations in the background.

jenswi-linaro commented 3 years ago

Update

jenswi-linaro commented 3 years ago

This rule is not applied in the Linux kernel, is it? This will make common definitions diverge a bit more (optee_msg.h, optee_rpc_cmd.h`) although I think these files are already not identical in both projects so it may not be an issue in practice.

It will result in even more manual work when synching those files with the kernel, but it is a not a big problem.

Also what about GP defines? (TEE_SUCCESS etc). I recall some discussion is ongoing in GP about the U suffix but I don't remember the status. I suppose we will do whatever GP requires if/when they update the spec.

In this case when using the U() macro it's more or less transparent to the code. I'd say that we should apply it here too, unless we can find something in the specification that would require us to not have an U suffix. Especially since TEE_Result is a uint32_t so TEE_SUCCESS and friends are clearly supposed to be unsigned.

jforissier commented 3 years ago

Acked-by: Jerome Forissier <jerome@forissier.org>

jenswi-linaro commented 3 years ago

Tag applied.

Thanks for the review. We're likely to need a few more changes as we try to align with Misra C guidelines. Is this a good way of discussing them? For more complicated or controversial changes perhaps it still makes sense to start here with a proposal that at least can be used to start a discussion.

jbech-linaro commented 3 years ago

Is this a good way of discussing them?

Works for me. LOC meeting is probably also an alternative for more lengthy discussions.

jforissier commented 3 years ago

Is this a good way of discussing them?

Works for me. LOC meeting is probably also an alternative for more lengthy discussions.

+1

jbech-linaro commented 3 years ago

Thanks Jens and Jerome!