RFR: 8347707: Standardise the use of os::snprintf and os::snprintf_checked [v2]

Kim Barrett kbarrett at openjdk.org
Thu Aug 21 12:23:56 UTC 2025


On Thu, 21 Aug 2025 06:34:41 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> src/hotspot/share/runtime/os.hpp line 810:
>> 
>>> 808:   // Performs vsnprintf and asserts the result is non-negative (so there was not
>>> 809:   // an encoding error or any other kind of usage error).
>>> 810:   [[nodiscard]] static int vsnprintf(char* buf, size_t len, const char* fmt, va_list args) ATTRIBUTE_PRINTF(3, 0);
>> 
>> Consider moving the `ATTRIBUTE_PRINTF` to the front so all the attributes are together?
>> And maybe a line break between the attributes and the signature, just to avoid pushing the
>> signature way over to the right.
>
> I have done that and placed each attribute on its own line (we should have a style guide entry for this :) ). But I note that all the other ATTRIBUTE_PRINTF's are placed after the function.

What you've done here matches what I did when adding `[[noreturn]]` to `report_xxx` functions
in debug.hpp.  So I'm good with that.  The style guide already has this guidance:
https://github.com/openjdk/jdk/blame/02fe095d29994bec28c85beb6bf2a69b0f49b206/doc/hotspot-style.md#L1120-L1122
There's just lots of old, non-conforming code. :)
I forgot about that when I said "Consider moving", and should have pointed to the style guide.
Whether and where there should be line breaks is something the style guide leaves to authors and reviewers.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26849#discussion_r2290880828


More information about the hotspot-dev mailing list