Skip to content

Conversation

@iampi31415
Copy link
Contributor

@iampi31415 iampi31415 commented Oct 22, 2025

Supporting information for my fix is here https://docs.rs/cortex-m-rt/latest/cortex_m_rt/#sections-size

  1. .data does not belong to Flash
    • So I removed this from Flash description.
  2. vector_table does belong to Flash —in this arch at least (see link above).
    • So I added it to the Flash Description.

You don't get 2008 bytes otherwise, but now it does add up to 2008.

cc: @adamgreig since it's last person working in this repo.

Supporting information for my fix is here https://docs.rs/cortex-m-rt/latest/cortex_m_rt/#sections-size

Notice that —besides the mistake— as the text is written, it is unclear why the sum is 2008, since it is nowhere said that `vector_table` belongs to flash at least in this chip.

I don't link it because it may break in the future; but I could include it I you want it.
Copy link
Member

@BartMassey BartMassey left a comment

Choose a reason for hiding this comment

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

"In these examples the program will…" → "In this example the uploaded software will…"

I think it's confusing to show a .text size and then say that the program occupies more flash than that. Maybe this is better?

@iampi31415
Copy link
Contributor Author

iampi31415 commented Oct 23, 2025

Thanks for the review.

Just to be clear the main issue is that .data is not in Flash as said there, and the sum is not 2008B unless we add vector_table to the "Flash group".

I think it's confusing to show a .text size and then say that the program occupies more flash than that. Maybe this is better?

Not sure what you mean by "this" in. "Maybe this is better"?

text is used for the sum, .text for a section of the binary format. To show the text size aggregated already, we do cargo size --release instead of cargo size --release -- -A

Not sure that is confusing either.


Comment: In the page https://github.com/rust-embedded/ it says:

We are an official working group of the Rust language.

Notice that the link is broken (goes to non-existent page.)

@iampi31415 iampi31415 requested a review from BartMassey October 23, 2025 19:19
Copy link
Member

@BartMassey BartMassey left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@BartMassey
Copy link
Member

Yeah, I understood that the sections were wrong. Thanks much for both fixes.

The link that you reference is probably broken because the Rust Project is in the process of figuring out what to do with Working Groups. Please file an issue against the rust-embedded/wg repo and I'll try to remember to bring it up at the next REWG meeting.

@iampi31415
Copy link
Contributor Author

iampi31415 commented Oct 24, 2025

Yeah, I understood that the sections were wrong. Thanks much for both fixes.

You are welcome.

Please file an issue against the rust-embedded/wg repo and I'll try to remember to bring it up at the next REWG meeting.

Done.

@BartMassey BartMassey merged commit d791b97 into rust-embedded:master Oct 26, 2025
0 of 2 checks passed
@BartMassey
Copy link
Member

Sigh. CI for this repo seems to be borked somehow. I think bors is the problem here maybe?

I went ahead and merged the commit. Thanks much for your contribution.

@iampi31415
Copy link
Contributor Author

Sigh. CI for this repo seems to be borked somehow. I think bors is the problem here maybe?

I went ahead and merged the commit. Thanks much for your contribution.

Can not find much except that

"The job has exceeded the maximum execution time while awaiting a runner for 24h0m0s"

I realised that vector_table is missing a period . , it should be .vector_table so I will fix the typo.

@BartMassey
Copy link
Member

It's fine. I fixed the CI and will get the missing dot. Thanks much!

adamgreig added a commit that referenced this pull request Oct 27, 2025
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