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: Split private image API out of image.h #1821

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

Conversation

de-nordic
Copy link
Collaborator

image.h will now only contain applicaiton image related structures and defines.
The split has been done to reduce need for including extra headers, for example FIH support and mcuboot_config.h into software that needs the same defining as MCUboot but implements own functions.

@de-nordic de-nordic marked this pull request as ready for review September 28, 2023 17:16
@d3zd3z d3zd3z self-requested a review December 14, 2023 15:28
@de-nordic de-nordic marked this pull request as draft March 1, 2024 14:50
@nvlsianpu
Copy link
Collaborator

I'm fine with this split

@d3zd3z
Copy link
Member

d3zd3z commented Sep 9, 2024

I do like the idea of this split, even though it doesn't really change anything in the code, it at least makes things a little clearer.

I'd really like to see us trying to migrate the APIs in mcuboot to something a bit more modern. I'm thinking of something like how methods work in either Zig or Rust. Where we have clearly defined types, and functions that operate on those types, and not functions that kind of willy-nilly reach between the different types. Ideally, these types would contain the data they need for these methods, for example, a struct that does stuff on a partition, would contain the opened flash api pointer in it (instead of opening and closing every time we use the flash). Each of these structs would have a clear initializer and finalizer as well. Although this has to be done fairly explicitly in C, it is doable (and similar to how it works in Zig).

Getting to this point is a lot of work. But would make it so much easier to play with other ideas we have about supporting newer swap algorithms or newer algorithms for storing the status data.

@de-nordic
Copy link
Collaborator Author

I do like the idea of this split, even though it doesn't really change anything in the code, it at least makes things a little clearer.

I'd really like to see us trying to migrate the APIs in mcuboot to something a bit more modern. I'm thinking of something like how methods work in either Zig or Rust. Where we have clearly defined types, and functions that operate on those types, and not functions that kind of willy-nilly reach between the different types. Ideally, these types would contain the data they need for these methods, for example, a struct that does stuff on a partition, would contain the opened flash api pointer in it (instead of opening and closing every time we use the flash). Each of these structs would have a clear initializer and finalizer as well. Although this has to be done fairly explicitly in C, it is doable (and similar to how it works in Zig).

Getting to this point is a lot of work. But would make it so much easier to play with other ideas we have about supporting newer swap algorithms or newer algorithms for storing the status data.

Yeah, so this PR is part of small cleanups and restructuring of code; as you have stated we should have some rework to improve general structure of code, improve modularity and do some de-coupling.

image.h will now only contain applicaiton image related structures
and defines.
The split has been done to reduce need for including extra headers,
for example FIH support and mcuboot_config.h into software that
needs the same defining as MCUboot but implements own functions.

Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
@de-nordic de-nordic marked this pull request as ready for review September 12, 2024 10:45
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.

Aside from it not building, I do think this is a move in the right direction.

@de-nordic
Copy link
Collaborator Author

Aside from it not building, I do think this is a move in the right direction.

Heh. I forgot I did that one. I will fix this and rebase.

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.

3 participants