Skip to content

Conversation

@addrian-77
Copy link

Pull Request Overview

This pull request fixes the values provided by the TBF Parser. init_fn_offset and protected_size also had the header_size added to them. In the python version version, the header_size is not added to them.

The TBF Version was wrongly obtained using the get_binary_version, which is different from the correct one, found in the base header.

TODO or Help Wanted

Checks

Using Rust tooling

  • Ran cargo fmt
  • Ran cargo clippy
  • Ran cargo test
  • Ran cargo build

Features tested:

  • info on microbit-v2 using tockloader-rs
  • tockloader info on microbit-v2

GitHub Issue

This pull request closes BUG: Differences in init_fn_size and protected_size fields for app TBF #70

@addrian-77 addrian-77 requested a review from eva-cosma December 14, 2025 21:16
@addrian-77 addrian-77 self-assigned this Dec 14, 2025
Signed-off-by: Adrian Lungu <lunguadrian30@gmail.com>
@addrian-77 addrian-77 force-pushed the fix/tbf-parser-values branch from 1ea68dd to 944cd9e Compare December 14, 2025 21:45
Copy link
Collaborator

@eva-cosma eva-cosma left a comment

Choose a reason for hiding this comment

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

From what I understand in tock/tock#3515 and https://book.tockos.org/doc/tock_binary_format?highlight=protected_trailer#header-tlvs , protected_size and protected_size_trailer are two different things. The whole size includes the header and the ..._trailer is without it. I would add a getter for the trailer and use that instead of modifying get_protected_size.

init_offset should be from the start of the binary, aka without the header, yet the rust library adds the header size? If my understanding is correct, than this is a misalignment between the documentation and the upstream repository (which we should report). In this scenario, I would modify the function as you did, BUT add to the function comment that we diverge from the upstream implementation AND add a README.md to our crate to note all the divergences from upstream. If my understanding is wrong, and the documentation says that it should include the header size, then let's not modify the function but rather just subtract the header when displaying it (and also raise to the tockloader team that they differ from the documentation).

See also tock/tockloader#125

Comment on lines +1172 to +1179
/// Return the TBF version
pub fn get_tbf_version(&self) -> u16 {
match self {
TbfHeader::TbfHeaderV2(hd) => hd.base.version,
_ => 0,
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to add this function?

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Thx

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.

BUG: Differences in init_fn_size and protected_size fields for app TBF

3 participants