Simplify message serialisation code and test#92
Merged
efd6 merged 2 commits intoelastic:mainfrom Feb 3, 2022
Merged
Conversation
Collaborator
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
adriansr
suggested changes
Oct 21, 2021
rule/binary.go
Outdated
| if out.Len()%4 > 0 { | ||
| out.Write(make([]byte, 4-(out.Len()%4))) | ||
| } | ||
| const ruleHeaderSize = int(unsafe.Sizeof(auditRuleData{}) - unsafe.Sizeof([]byte(nil))) |
Contributor
There was a problem hiding this comment.
I understand the header here means everything in auditRuleData except the Buf []byte field.
This feels a little unsafe, what about splitting auditRuleData in two separate structs, for example auditRuleHeader with the header fields and a new auditRuleData made of the struct composition of auditRuleHeader and the internal-use fields. This way there's less risk of forgetting to subtract a new field.
Contributor
Author
|
PTAL |
Conventionally Go eschews use of unsafe byte ordering tricks for message serialisation but given that the kernel makes use of them, and we need to use one to determine which ordering to use in encoding/binary anyway, we may as well use them.
renini
pushed a commit
to renini/go-libaudit
that referenced
this pull request
Jun 26, 2024
Conventionally Go eschews use of unsafe byte ordering tricks for message serialisation but given that the kernel makes use of them, and we need to use one to determine which ordering to use in encoding/binary anyway, we may as well use them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reduces the load on encoding/binary.Read and others at the cost of using unsafe. Given that we are close to the border of the kernel here, this is pretty reasonable. Testing is increased to ensure that message serialisation is at least round-trippable.
Please take a look.