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