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