RFR: 8314114: Fix -Wconversion warnings in os code, primarily linux [v2]
Coleen Phillimore
coleenp at openjdk.org
Thu Aug 10 22:52:33 UTC 2023
On Thu, 10 Aug 2023 21:38:28 GMT, Dean Long <dlong at openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
>>
>> - Dean's suggested changes.
>> - Merge branch 'master' into os-conversion
>> - Fix cgroupSubsystem_linux.cpp
>> - Unset Wconversion for testing.
>> - Fix signals to take int buflen, fixed return types of syscalls
>> - Fix attachListener.cpp warnings.
>> - change send/recv parameter type to match OS.
>> - Fix -Wconversion warnings in os_linux.
>
> src/hotspot/os/linux/os_linux.cpp line 3204:
>
>> 3202: for (int j = 0; j < cpu_map_valid_size; j++) {
>> 3203: if (cpu_map[j] != 0) {
>> 3204: for (int k = 0; k < BitsPerCLong; k++) {
>
> For changes to lines 3145 to 3204, wouldn't it be better to keep using size_t?
I started that way and it required more casts. This was the path of least cast-ness.
> src/hotspot/os/linux/systemMemoryBarrier_linux.cpp line 59:
>
>> 57: };
>> 58:
>> 59: static int64_t membarrier(int cmd, unsigned int flags, int cpu_id) {
>
> Seems OK, but if syscall returns `long` then intrptr_t seems a better fit for 32-bit platforms. Or we could narrow to int here, as long as membarrier (and futex) aren't returning pointers or memory addresses.
I think intptr_t is a better choice.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15229#discussion_r1290740835
PR Review Comment: https://git.openjdk.org/jdk/pull/15229#discussion_r1290741805
More information about the hotspot-dev
mailing list