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

manifest: ensure that size always defaults to -1 in RaucImage #1384

Merged
merged 1 commit into from Apr 10, 2024

Conversation

jluebbe
Copy link
Member

@jluebbe jluebbe commented Apr 8, 2024

A size of -1 indicates that no size key was given in the manifest. Instead of hardcoding that in everywhere, create a r_new_image() constructor function and use it instead.

@ejoerns
Copy link
Member

ejoerns commented Apr 9, 2024

A size of -1 indicates that no size key was given in the manifest. Instead of hardcoding that in everywhere, create a r_new_image() constructor function and use it instead.

This does not replace any hard-coded setting of checksum.size as far as I can see (what the commit message suggests it does). So does this fix potential issues?

@jluebbe
Copy link
Member Author

jluebbe commented Apr 9, 2024

A size of -1 indicates that no size key was given in the manifest. Instead of hardcoding that in everywhere, create a r_new_image() constructor function and use it instead.

This does not replace any hard-coded setting of checksum.size as far as I can see (what the commit message suggests it does). So does this fix potential issues?

I have some patches in my stack that perform stricter checking of input manifests. They enforce that no checksum or size is set, so a default constructed RaucImage would have size 0 and fail (especially in the test cases).

But even right now it's inconsistent, as a default RaucImage would be written to a manifest with size=0.

@ejoerns
Copy link
Member

ejoerns commented Apr 9, 2024

So with mentioning the inconsistency in the commit message I'd be happy ;)

A size of -1 indicates that no size key was given in the manifest. Right
now it's inconsistent, as a default RaucImage would be written to a
manifest with size=0. Instead of setting -1 everywhere for consistency,
create a r_new_image constructor function and use it instead.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
@jluebbe
Copy link
Member Author

jluebbe commented Apr 9, 2024

@ejoerns I've updated the commit message.

Copy link
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

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

I just wanted to add that using r_rauc_image_free() or r_image_free() would better fit our naming conventions, but then noted that we have r_free_image() already. Thus we should better rename these together at any later point.

@ejoerns ejoerns merged commit ea6f905 into rauc:master Apr 10, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants