RFR: 8360178: TestArguments.atojulong gtest has incorrect format string

[David Holmes]

On Mon, 23 Jun 2025 02:05:29 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> Please review this fix for the format directive when printing ULLONG_MAX by
> TestArguments.atojulong gtest. Instead of using JULONG_FORMAT, which might not
> be the proper type (though sized properly), use "%llu" to avoid potential
> warning.
> 
> Also changed the test_arguments.cpp file to use os::snprintf instead of
> jio_snprintf, and improved checking the results of those calls.
> 
> Testing: mach5 tier1

The change to "%llu", would be trivial, but the rest seems a bit superfluous.

test/hotspot/gtest/runtime/test_arguments.cpp line 32:

> 30: #include <errno.h>
> 31: 
> 32: #include "unittest.hpp"

Is there a rule/guideline that says this should be placed here?

test/hotspot/gtest/runtime/test_arguments.cpp line 51:

> 49:     char buf[1024];
> 50:     int ret = os::snprintf(buf, sizeof(buf), "%s=%s", name, value);
> 51:     if ((ret > 0) && ((size_t)ret < sizeof(buf))) {

`jio_snprintf` already handled the buffer-too-small case by returning -1. I don't see the reason for changing the test to use `os::snprintf` when arguments.cpp uses `jio_snprintf`.

test/hotspot/gtest/runtime/test_arguments.cpp line 63:

> 61:   int ret = os::snprintf(ullong_max, sizeof(ullong_max), "%llu", ULLONG_MAX);
> 62:   ASSERT_LT(0, ret);
> 63:   ASSERT_LT(ret, (int)sizeof(ullong_max));

This is just a sanity check right? Maximum uint64_t has 20 digits.

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

PR Review: https://git.openjdk.org/jdk/pull/25927#pullrequestreview-2948976151
PR Review Comment: https://git.openjdk.org/jdk/pull/25927#discussion_r2160914674
PR Review Comment: https://git.openjdk.org/jdk/pull/25927#discussion_r2160929332
PR Review Comment: https://git.openjdk.org/jdk/pull/25927#discussion_r2160920953