[PATCH] D26477: Try to make ScopedPrinter use the new binary blob formatter
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 9 15:08:58 PST 2016
zturner created this revision.
zturner added reviewers: mehdi_amini, clayborg.
zturner added a subscriber: llvm-commits.
`ScopedPrinter` was already using its own algorithm to format binary blob data. With the new method added by clayborg, I wanted to get down to a single implementation.
In the process of doing this, I needed to update the code a little bit to support some things that `ScopedPrinter` needed to do. Here are the material changes from the previous implementation:
1. Changed name from `format_hex_bytes` to `format_binary`. I felt `format_hex_bytes` was too similar to `format_hex`.
2. Added the ability to offset each line of the binary blob by an initial offset.
3. Fixed a bug in the implementation where we were calling the equivalent of `isprint((char)Byte)`, which can cause the argument to `isprint()` to be negative, which is undefined behavior.
4. Got a little smarter about how wide to print the offset field. Previously we were just using a full 8 bytes for the offset field, but with this patch I calculate the maximum offset value we will need to print, determine how many nibbles are needed to print that value, and then align all offset values to that width.
5. Remove the `0x` prefix from the offset. I could have added an option to choose whether we want the prefix, and if anyone really needs that I can add it back. I removed it because already have a lot of tests that will fail if the offset is there, and keeping the API simple seems desirable if nobody really needs that functionality.
6. Made a few things more idiomatic / LLVMish. For example, `None` instead of a default constructed `Optional<uint64_t>()`, remove null checking from unit test helper functions (pointless since we control all entry points into the function), changing function signatures to accept `ArrayRef` instead of `const void *` and length.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 22520 bytes
Desc: not available
More information about the llvm-commits