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