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