RFR: 8292981: Unify and restructure integer printing format specifiers [v10]

Kim Barrett kbarrett at openjdk.org
Thu Sep 1 22:13:15 UTC 2022


On Thu, 1 Sep 2022 16:33:13 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Today we have some inconsistencies in how we name our integer printing format specifiers. I'd like to change this to be consistent. 
>> 
>> This patch comes from a discussion in #10028, which snowballed into restructuring the format specifiers. The main issues was that my original patch used PTR<size>_FORMAT to print integers with the format 0x000<value>. The reviewers felt that it was wrong to use PTR format specifiers when printing integers. I agree with that.
>> 
>> We do have format specifiers to print hex values out of integers, though they don't 0-pad like the PTR macros do, and only some of the prepend 0x.
>> 
>> I'd like to suggest that we use a convention to specify what we want. This is the current proposal:
>> 
>> // Guide to the suffixes used in the format specifiers for integers:
>> //        - print the decimal value:                   745565
>> //  _X    - print as hexadecimal, without leading 0s: 0x12345
>> //  _X_0  - print as hexadecimal, with leading 0s: 0x00012345
>> //  _H    - print as hexadecimal, without 0x prefix
>> //  _W(w) - prints w sized string with the given value right
>> //          adjusted. Use -w to print left adjusted.
>> //
>> // Note that the PTR format specifiers print using 0x with leading zeros,
>> // just like the _X_0 version for integers.
>> 
>> 
>> The patch also removes PTR32_FORMAT and PTR64_FORMAT and replace them with the corresponding integer format specifiers.
>
> Stefan Karlsson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix gtest compilation warning

Thanks for adding a gtest.  One new comment that you can do whatever you want with.

src/hotspot/share/cds/filemap.cpp line 1556:

> 1554:   if (size > 0) {
> 1555:     log_info(cds)("Shared file region (%-3s)  %d: " SIZE_FORMAT_W(8)
> 1556:                    " bytes, addr " INTPTR_FORMAT " file offset 0x%08" PRIxPTR

If not using one of our FORMAT macros for this one-off place, I think
"0x%08zx" is shorter and at least as understandable as using of one of the PRI
macros (whose meanings I can never remember, and whose very existence I tend
to forget, but maybe that's just me).

But really, I wonder what the point of the field width is here?  Why isn't
this just SIZE_FORMAT_X?

Alternatively, maybe SIZE_FORMAT_X_W should be SIZE_FORMAT_X_0_W?

-------------

Marked as reviewed by kbarrett (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10042


More information about the hotspot-dev mailing list