RFR: 8214976: Warn about uses of functions replaced for portability [v2]

Harold Seigel hseigel at openjdk.java.net
Wed Jan 5 20:41:10 UTC 2022


On Wed, 5 Jan 2022 03:21:54 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   revert strdup() changes and address some of Kim's comments
>
> src/hotspot/os/linux/gc/z/zMountPoint_linux.cpp line 37:
> 
>> 35: #define PROC_SELF_MOUNTINFO        "/proc/self/mountinfo"
>> 36: 
>> 37: PRAGMA_PERMIT_FORBIDDEN_C_FUNCTION(fopen);
> 
> The intended usage for this pragma is to scope it narrowly, using `PRAGMA_DIAG_PUSH/POP`.

Fixed.

> src/hotspot/share/jvmci/jvmciCodeInstaller.cpp line 511:
> 
>> 509:       JVMCI_ERROR_OK("stub should have a name");
>> 510:     }
>> 511:     char* name = os::strdup(jvmci_env()->as_utf8_string(stubName));
> 
> Another `os::strdup` that I'm not sure is correct because I'm not sure where corresponding `free` might be, and whether it is `::free` or `os::free`.

Fixed by reverting strdup() changes.

> src/hotspot/share/runtime/os.cpp line 93:
> 
>> 91: #endif
>> 92: 
>> 93: PRAGMA_PERMIT_FORBIDDEN_C_FUNCTION(fopen); // prevents compiler warnings for all functions
> 
> Scope should be limited to the implementation of `os::fopen`.

Fixed

> src/hotspot/share/utilities/compilerWarnings.hpp line 87:
> 
>> 85:   PRAGMA_DISABLE_GCC_WARNING("-Wattribute-warning")
>> 86: 
>> 87: FORBID_C_FUNCTION(void abort(void),                            "use os::abort");
> 
> It would be better to put all of these after all the `#endif`, so that if we add macro implementations for other platforms (like windows), these will be covered by the additional platforms.

Done.

> src/hotspot/share/utilities/compilerWarnings.hpp line 112:
> 
>> 110: 
>> 111: #else
>> 112: 
> 
> I think, but have not tested it, that this facility can be implemented for Visual Studio using `__declspec(deprecated)` and suppressing warning C4996.  Of course, doing that may trigger a bunch of warnings in Windows-specific files, so it might be best to do that as a followup change.

Addressing this as a follow up change sounds good.

> test/hotspot/gtest/logging/test_logDecorators.cpp line 29:
> 
>> 27: #include "unittest.hpp"
>> 28: 
>> 29: PRAGMA_PERMIT_FORBIDDEN_C_FUNCTION(strdup);
> 
> Why suppress this warning?  Why not just fix the couple of calls, and remember to also fix the corresponding calls to `::free` to instead call `os::free`.

Fixed.

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

PR: https://git.openjdk.java.net/jdk/pull/6961


More information about the hotspot-dev mailing list