Skip to content

sys/suit: write size in suit_worker_try_prepare()#21913

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
benpicco:suit_worker_try_prepare-set_size
Nov 25, 2025
Merged

sys/suit: write size in suit_worker_try_prepare()#21913
maribu merged 1 commit intoRIOT-OS:masterfrom
benpicco:suit_worker_try_prepare-set_size

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Nov 24, 2025 •
edited
Loading

Contribution description

suit_worker_try_prepare() is a bit awkward to use.
The idea is that we can request the buffer into which the SUIT manifest shall be written.
However it only tells us the size of the destination buffer if we call it with a size larger than the destination buffer.

Otherwise the caller has to either guess the size or call suit_worker_try_prepare() twice.

uint8_t *mani_buf;
size_t mani_size_max = UINT32_MAX;

suit_worker_try_prepare(&mani_buf, &mani_size_max); // get size
suit_worker_try_prepare(&mani_buf, &mani_size_max); // get buffer

Just populate size at all times to avoid such nonsense.

Testing procedure

Issues/PRs references

@github-actions github-actions bot added Area: OTA Area: Over-the-air updates Area: sys Area: System labels Nov 24, 2025
@benpicco benpicco requested review from chrysn and maribu November 24, 2025 18:18
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 24, 2025
@maribu
Copy link
Copy Markdown
Member

maribu commented Nov 24, 2025

I wonder if existing users might rely on the value of *size not being changed on success. E.g. when the manifest size is known already at the point in time suit_worker_try_prepare() is called and the caller just passes that value first to suit_worker_try_prepare(), and then to some copy operation under the assumption size has not been increased?

Or can that be ruled out?

@maribu maribu added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Nov 24, 2025
@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Nov 24, 2025

I can't find any existing users in-tree.
@chrysn added this, do you still know the rationale behind this?

We can of course make this conditional to size being passed as 0.

@maribu
Copy link
Copy Markdown
Member

maribu commented Nov 24, 2025

If *size is only overwritten when it was 0 before, I don't see a way this can break existing valid users.

@benpicco benpicco force-pushed the suit_worker_try_prepare-set_size branch from 46fb74e to 237fe94 Compare November 24, 2025 18:46
@benpicco benpicco force-pushed the suit_worker_try_prepare-set_size branch from 237fe94 to 8aa816a Compare November 24, 2025 18:47
@maribu maribu removed the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Nov 24, 2025
@benpicco benpicco changed the title sys/suit: always write size in suit_worker_try_prepare() sys/suit: always size in suit_worker_try_prepare() Nov 24, 2025
@maribu maribu added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Nov 24, 2025
@benpicco benpicco changed the title sys/suit: always size in suit_worker_try_prepare() sys/suit: write size in suit_worker_try_prepare() Nov 24, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Nov 24, 2025

Murdock results

✔️ PASSED

8aa816a sys/suit: always write size in suit_worker_try_prepare()

Success Failures Total Runtime
10932 0 10932 12m:32s

Artifacts

@maribu maribu added this pull request to the merge queue Nov 25, 2025
Merged via the queue into RIOT-OS:master with commit 663f52c Nov 25, 2025
28 checks passed
@benpicco benpicco deleted the suit_worker_try_prepare-set_size branch November 25, 2025 13:55
@leandrolanzieri leandrolanzieri added this to the Release 2026.01 milestone Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maribu maribu maribu approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

@chrysn chrysn Awaiting requested review from chrysn

Assignees

No one assigned

Labels

Area: OTA Area: Over-the-air updates Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2026.01

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @maribu @riot-ci @leandrolanzieri