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

Represent big integers as arrays of bytes #87

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

barsnick
Copy link
Contributor

@barsnick barsnick commented Oct 24, 2024

Describe your changes

In PR #65, support for proper coding of large integer (xs:integer) was added, aiding in the proper handling of X509SerialNumber.

Yet the API presented those numbers in the same style as EXI internally, i.e. with 8/7 padding, adding an indicator MSB to each octet for understanding whether the sequence continues.

This change now expects those numbers as arrays of bytes (still within an exi_unsigned_t), representing the integer, without any padding. The byte order is big-endian, i.e. leading bytes in the array have the most significant meaning.

Issue ticket number and link

Relates to issue #54
Amends PR #65

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

Signed-off-by: Moritz Barsnick <[email protected]>
Signed-off-by: Moritz Barsnick <[email protected]>
For code readability.

Signed-off-by: Moritz Barsnick <[email protected]>
When adding the final byte or octet, skip it when its value is zero.
Trailing 0-octets are meaningless in both cases and shouldn't be provided.

Signed-off-by: Moritz Barsnick <[email protected]>
Found by review, confirmed by cppcheck.

Also reorder the code lines to a more logical order.

Signed-off-by: Moritz Barsnick <[email protected]>
Signed-off-by: Moritz Barsnick <[email protected]>
@barsnick
Copy link
Contributor Author

barsnick commented Oct 24, 2024

Missing:

  • Separate array sizes (and possibly types) for the raw 8-bit representation and the padded 8/7-bit representation
  • Documentation of this particular API
  • Tests/unit tests

@barsnick barsnick changed the title Represent big integers as array of bytes Represent big integers as arrays of bytes Oct 24, 2024
Copy link
Collaborator

@SebaLukas SebaLukas 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. Maybe I have found one small thing.
But I have not yet generated the code and tested it against the libcbv2g unit tests

}
if (dummy_count > 0) {
*(current_octet-1) |= EXI_BASETYPES_OCTET_SEQ_FLAG_MASK;
if (dummy_count > 0 && dummy != 0) {
Copy link
Collaborator

@SebaLukas SebaLukas Oct 24, 2024

Choose a reason for hiding this comment

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

Is dummy != 0 here necessary?
It's relevant, I just read dummy_count twice...

@SebaLukas
Copy link
Collaborator

libcbv2g with the generated code builds without problems. Unit tests passed and EVerest SIL work as well. So happy to merge this one 👍

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