RFR: JDK-8293711: Factor out size parsing functions from arguments.cpp [v2]
Stefan Karlsson
stefank at openjdk.org
Wed Sep 21 05:23:31 UTC 2022
On Tue, 20 Sep 2022 15:23:58 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Arguments.cpp has several size parsing functions which would be useful in other areas of the hotspot, e.g. in NMT.
>>
>> It would be nice to have them factored out into utilities, to reuse the code and to unify memory size handling. Gtests would be good too.
>>
>> To simplify reviews, I split the patch into two commits.
>>
>> The first commit (https://github.com/openjdk/jdk/pull/10252/commits/700e77e8d1469a2fc3d6611072c4b07aa34ab8e6) contains the unchanged code move without functional changes.
>>
>> The second commit (https://github.com/openjdk/jdk/pull/10252/commits/76b4f6f30cc316fd966da60ff6601f54eeb394bf) contains the functional changes I did, as well as the new gtest.
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>
> feedback johan
Changes requested by stefank (Reviewer).
src/hotspot/share/utilities/globalDefinitions.hpp line 113:
> 111: // Format 32-bit quantities.
> 112: #define INT32_FORMAT "%" PRId32
> 113: #define INT32_FORMAT_X "0x%" PRIx32
Could you add corresponding test lines to test_globalDefintions.cpp?
src/hotspot/share/utilities/parse_integer.hpp line 2:
> 1: /*
> 2: * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
I think you should use the copyright dates from the file where this code got copied from. Same goes for the other new file.
src/hotspot/share/utilities/parse_integer.hpp line 93:
> 91: }
> 92:
> 93: #endif // SHARE_UTILITIES_COPY_HPP
Wrong comment
test/hotspot/gtest/testutils.hpp line 60:
> 58:
> 59: #define LOG_HERE(s, ...) { printf(s, __VA_ARGS__); printf("\n"); fflush(stdout); }
> 60:
In other reviews I've asked the reviewers to use unified logging with the `test` tag. Maybe do the same for this test?
-------------
PR: https://git.openjdk.org/jdk/pull/10252
More information about the hotspot-dev
mailing list