On Thu, 21 Aug 2025 06:34:41 GMT, David Holmes <dhol...@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
