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

Increase size of file writing #78

Closed
wants to merge 2 commits into from
Closed

Increase size of file writing #78

wants to merge 2 commits into from

Conversation

sosthene-nitrokey
Copy link
Contributor

The increased size of files is a requirement for PIV, 1kB certificates are too small. This commit also removes the user_attribute field that was never read anyway. This avoids increasing the total size of the interchange

Both changes are breaking changes:

  • Changing the messages size for the read and write requests can cause compilation error for code that depends on the length of the heapless arrays
  • Removing user_attribute will require changing every call to the read_file and write_methods to remove the None.

@nickray
Copy link
Member

nickray commented Jan 31, 2023

Yeah, we can remove UserAttribute. If I remember correctly, it would have allowed filtering a tree of files without loading the files themselves, which might be an optimization for apps with a large amount of such files. So since we don't use it, can add back later if and only if someone actually needs this.

The increased size of files is a requirement for  PIV,
1kB certificates are too small.
The motivation for this change is to avoid increasing the interchange size
with the larger data for files.

The justification for removing it are the following:
- They are not used anywhere
- They were actually never written, so even if someone used them they did nothing when writing
- Their usage was not documented at all
@sosthene-nitrokey
Copy link
Contributor Author

This will we replaced by the streaming api. Current advances can be found in Nitrokey#18. Once #96 and trussed-dev/littlefs2#34 are merged I will upstream it.

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