RFR: 8334738: os::print_hex_dump should optionally print ASCII [v2]
Thomas Stuefe
stuefe at openjdk.org
Tue Jun 25 12:51:13 UTC 2024
On Tue, 25 Jun 2024 07:05:47 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Thomas Stuefe has updated the pull request incrementally with five additional commits since the last revision:
>>
>> - copyrights
>> - const_address instead of const uint8_t*
>> - use dot instead of underscore for unprintable
>> - rely on default true
>> - Revert "fix copyrights"
>>
>> This reverts commit 2b8bc55e53a88d13cd268dc89ebaac7fe42f60d5.
>
>> I got tired of casting constness away from to-be-printed memory range just to be able to feed an address to os::print_hex_dump.
>
> I don't follow - you didn't change/add any callsites so why would you need to cast away constness to "feed" the existing call ??
>
> More comments below.
>
> I like the idea in general.
Hi @dholmes-ora, thanks for your review!
I hopefully addressed all your concerns. In particular:
- I now rely on `true` for the default of print_ascii, and rolled back changes to all callsites that explicitly passed true. I also decorated the parameters with comments
- I changed read_safely_from to take an uintptr_t* pointer. I would like to retain the signed->unsigned change, otherwise I would have to cast more in the 32-bit path.
- I introduced a new `const_address` typedef to complement the ubiquitous `address`. If we have one, we should have the other.
- I now use a dot instead of an underscore for unprintable data. This seems to be the default most hex editors use.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19835#issuecomment-2188846678
More information about the hotspot-dev
mailing list