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

boot/bootutil: add the option to use custom definitions for all crypto inline functions #1870

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Olstyle
Copy link

@Olstyle Olstyle commented Nov 30, 2023

As previously discussed in #1478 , this adds a define named MCUBOOT_USE_CUSTOM_CRYPT.
Using that define, headername_custom.h will be included in the crypto headers instead of enabling any pre defined code path.
Since the include calls throughout mcuboot are clean already, it is actually enough to implement only the used algorithms. So if e.g. RSA is not used, a definition of rsa_custom.h is NOT necessary.

…ons instead of the prepared ones as previously discussed in mcu-tools#1478

Signed-off-by: Oliver Mueller <olstyle@gmx.de>
@Olstyle Olstyle changed the title add the option to use custom definitions for all crypto inline functions boot/bootutil: add the option to use custom definitions for all crypto inline functions Nov 30, 2023
@d3zd3z d3zd3z self-requested a review December 14, 2023 15:30
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the stale label Jun 12, 2024
@Olstyle
Copy link
Author

Olstyle commented Jun 12, 2024

Please don't activate an auto close bot who is faster in handling issues than the maintainers.

@github-actions github-actions bot removed the stale label Jun 13, 2024
Copy link
Member

@d3zd3z d3zd3z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't quite clear to me the benefit of adding a set of "custom" headers that aren't included, instead of just allowing what the header that should be used itself to be a define? Then the two existing ones wouldn't be special, other than being in tree, and custom ones can be named appropriately by whoever is making them.

@Olstyle
Copy link
Author

Olstyle commented Nov 13, 2024

It isn't quite clear to me the benefit of adding a set of "custom" headers that aren't included, instead of just allowing what the header that should be used itself to be a define? Then the two existing ones wouldn't be special, other than being in tree, and custom ones can be named appropriately by whoever is making them.

This is merely the smallest change I could think of without breaking the current way of configuring the crypto library, making this a minor change.

From a functional point of view one could get rid of the library define all together and just have multiple versions of the crypto headers in separate folders. Configuration would be done by including the "glue header" folder of the desired library.
But of course this is then a major version change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants