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

Implement the base of a TBF Parser #7

Closed
wants to merge 13 commits into from

Conversation

george-cosma
Copy link
Contributor

@george-cosma george-cosma commented Jul 30, 2023

In this PR, I am aim to implement a basic parser based on the one from tock-tbf. I want this to be a base to work off of in the future, as tockloader-rs is being developed.

To sum up briefly what I have done in this PR:

  • created a workspace, with "tbf-parser" as a member
  • copied over the code from tock-tbf into this new crate
  • modified data types so that they no longer require a static-lifetime u8 buffer.
    • This commit contains the changes.
    • As a note, as it currently is, this means that the buffer in which we read the flash should live as long as (or longer) than the parsed header.
  • Added a simple header as a test.

TODO:

  • Fix clippy errors
  • Parse footer from a non-static lifetime buffer

@george-cosma
Copy link
Contributor Author

george-cosma commented Aug 7, 2023

Update 1

I have made the footer parser work without static-lifetime byte slices. The 'try_into` implementation was a bit more annoying to get right, but what I did boils down to explicitly allowing casting only in the case where the byte array lives longer than the Footer.

Proposal

I did have an idea though, I could split off the different footer types (sha256, etc.) into their own structs with a fixed-sized slice. If we know we have a Sha256 footer we also know the u8 slice needs to be 32 long. And so on. This way, no need to worry about lifetimes, we just take ownership of the data. And if we do go this route, we could also enforce a maximum size limit on the package name (say 256 bytes) and we would have no lifetime limitations, we could ditch the original data altogether after parsing the footer. What do you people think?

Details for commit here: e989e86

Update 2

I have fixed the clippy warnings, some were trivial like unnecessary casting, or replacing match with an Option.map. One issue was regarding the huge size difference in the TbfHeader enum, but to me this doesn't seem like that big of an issue and in any case, boxing seems to be out of the question since we are avoiding std. Also, I have decided to just allow redundant field names. For me, even if data: data is a bit repetitive, it is clearer what's happening.

Details here: 5f5fcf2

In these two updates, I have achieved most of what I wanted, I believe I have a pretty robust base down for the tbf-parser. I'm opening this up for review.

@george-cosma george-cosma marked this pull request as ready for review August 7, 2023 18:14
@george-cosma
Copy link
Contributor Author

In the meantime I have worked on my previous proposal, and managed to get the TBF Header to fully own all of its data, meaning the original data can be abandoned altogether after parsing. This does however come at the cost of increased size in RAM for both the header and the footer.

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