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

Manually add hex prefix when formatting an Operand #1313

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

bbiiggppiigg
Copy link
Collaborator

I believe this is a misuse of snprintf format string.
Instead of outputting the numeric value in hex, it would output the value as dec followed by a literal x which makes no sense at all.

@bbiiggppiigg bbiiggppiigg self-assigned this Oct 25, 2022
@bbiiggppiigg bbiiggppiigg added bug instructionAPI This issue is directly related to instructionAPI labels Oct 25, 2022
Copy link
Contributor

@kupsch kupsch left a comment

Choose a reason for hiding this comment

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

Slight improvement would be to use "%#lx" instead of "0x%lx".

@hainest
Copy link
Contributor

hainest commented Oct 26, 2022

Slight improvement would be to use "%#lx" instead of "0x%lx".

I was going to recommend that, but it looks like it doesn't append the prefix when the value is zero. A quick grep found that other usages manually prepend the "0x". I don't know if it that is to handle the zero case or just from copy/paste.

@kupsch
Copy link
Contributor

kupsch commented Oct 26, 2022

If you want "0x0" then leave the code as is. If just "0" is acceptable then use the '#' modifier. I could see either case being acceptable.

@hainest hainest changed the title Fix Operand::format when pc value binding happens Manually add hex prefix when formatting an Operand Nov 9, 2022
@hainest hainest merged commit 1e37901 into master Nov 9, 2022
@bbiiggppiigg bbiiggppiigg deleted the fix_operand_format branch October 3, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug instructionAPI This issue is directly related to instructionAPI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants