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

Enable compile-time checks (parameter type-safety) for similar functions #176

Open
henrygab opened this issue Dec 28, 2024 · 0 comments
Open
Labels
enhancement New feature or request

Comments

@henrygab
Copy link
Collaborator

henrygab commented Dec 28, 2024

There are two APIs which both take a pin:

bool system_pin_claim(bool enable, uint8_t pin,     enum bp_pin_func func, const char* label);
bool system_bio_claim(bool enable, uint8_t bio_pin, enum bp_pin_func func, const char* label);

The only difference between the two is that one is a "system" pin, and the other is a "bio" pin.

Having such similar APIs, where the parameter type is identical (uint8_t) for subtly different types, makes it easy to have coding errors.

possible ways to improve

C is notoriously difficult to add type-safety, in part because of it's willingness to silently convert enum values into any form of integer large enough to store the value. The only place C retains the enum type is within a struct. This does raise the option of defining, for each enum type where type safety is desired, a corresponding structure whose sole field is the enum. Here's a quick summary of some of the benefits / drawbacks....

Drawbacks

  • Requires defining each enum value twice. Once in the enum, and once to define the structure that has that enum value as its only field. There is no simple way use macros to define this ... and thus is a potential copy/paste error during defining these values.

Neutral

  • Can define the structure instances as #define ... which makes it easier for compiler to inline everything.
  • Can publicly define the enum type without defining its values; the structure instances can be defined as extern const ... which allows hiding the enum type values (preserving namespace outside the compilation unit that defines the constants). Link-time optimization for current generation compilers still optimizes this equivalently.

Benefits

  • Functions that previously took an enum or integer now must receive a type-safe parameter value. This avoids, at compilation time, an entire class of coding errors.

Example

See definition of bp_debug_should_print(), and how the flags vs. levels were defined to prevent their being swapped in calling this function. In the above example, the extra step of declaring the values using extern const, and then instantiating them only in debug_rtt.c, was not used .... instead just #define'ing the values using typecasts.

Recommended

Define an extern const BIO_STRUCTURE_TYPE bio[] that converts from an integer to the type-safe type.

Most likely, the above changes would be done in the existing src/platform/bpi-rev10.h and similar, which already have #define values for BUFDIR0..BUFDIR7 and BUFIO0..BUFIO7, and which already defines an extern const uint8_t bio2bufiopin[8] and extern const uint8_t bio2bufdirpin[8]. Thus, the defines for these (and the extern instances) are already in existence, which should simplify the change.

@henrygab henrygab added the enhancement New feature or request label Dec 28, 2024
@henrygab henrygab changed the title Enable additional type-safety for similar functions Enable compile-time checks (parameter type-safety) for similar functions Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant