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

asm: add Instructions.SizeBytes() for convenience and readability #492

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Nov 17, 2021

This fixes a subtle bug where buf would always be reallocated if the program contains dword loads.

@ti-mo ti-mo requested a review from lmb November 17, 2021 14:05
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Maybe you've seen this already, there is asm.RawInstructionOffset which serves a similar purpose. It feels like there is some overlap, but no clever ideas come to me. Maybe you have one.

asm/instruction.go Outdated Show resolved Hide resolved
@lmb
Copy link
Collaborator

lmb commented Nov 17, 2021

This fixes a subtle bug where buf would always be reallocated if the program contains dword loads.

Super nit: not a bug imo, everything still works :P

@ti-mo
Copy link
Collaborator Author

ti-mo commented Nov 17, 2021

This fixes a subtle bug where buf would always be reallocated if the program contains dword loads.

Super nit: not a bug imo, everything still works :P

Yup, but if insns has a single dword, the size of the buffer will invariably be doubled, which isn't great. (for performance)

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo merged commit 1743a89 into cilium:master Nov 17, 2021
@ti-mo ti-mo deleted the tb/asm-size-bytes branch November 17, 2021 16:18
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.

None yet

2 participants