RFR: 8330989: unify os::create_binary_file across platforms [v2]
David Holmes
dholmes at openjdk.org
Wed May 1 08:57:55 UTC 2024
On Tue, 30 Apr 2024 16:20:20 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>>> > Hi David, no compile or test errors have been observed in GHA or in our internal testing. So should we keep what we have? Or go for the '_' prefixed flags and _open on Windows ?
>>>
>>> Okay. Based on this:
>>>
>>> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/open?view=msvc-170 https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170#posix-function-names
>>>
>>> I expected to see compiler deprecation warningsm as AFAICS we don't disable them. ??
>>>
>>> If this works then I guess it is okay.
>>
>> Hi David, this is indeed a little strange. The MS document you mentioned says
>>
>>> The Microsoft-implemented POSIX function name open is a deprecated alias for the [_open](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/open-wopen?view=msvc-170) function. By default, it generates [Compiler warning (level 3) C4996](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170).
>>
>> We disable the mentioned warning 4996 for quite a few compilation units (grep for 4996 in make/modules), but not for all.
>> I do not see it disabled for os.cpp. But still no compilation errors in GHA or our builds/tests. Maybe for some strange reason 'open' is bad but '::open' is okay for MSVC (it is used anyway in os_windows.cpp see `os::dll_load`)?
>
>> > > Hi David, no compile or test errors have been observed in GHA or in our internal testing. So should we keep what we have? Or go for the '_' prefixed flags and _open on Windows ?
>> >
>> >
>> > Okay. Based on this:
>> > https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/open?view=msvc-170 https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170#posix-function-names
>> > I expected to see compiler deprecation warningsm as AFAICS we don't disable them. ??
>> > If this works then I guess it is okay.
>>
>> Hi David, this is indeed a little strange. The MS document you mentioned says
>>
>> > The Microsoft-implemented POSIX function name open is a deprecated alias for the [_open](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/open-wopen?view=msvc-170) function. By default, it generates [Compiler warning (level 3) C4996](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-170).
>>
>> We disable the mentioned warning 4996 for quite a few compilation units (grep for 4996 in make/modules), but not for all. I do not see it disabled for os.cpp. But still no compilation errors in GHA or our builds/tests. Maybe for some strange reason 'open' is bad but '::open' is okay for MSVC (it is used anyway in os_windows.cpp see `os::dll_load`)?
>
> Not a review, just a drive-by comment.
>
> For Windows builds we define _CRT_NONSTDC_NO_DEPRECATE in the build system.
> https://github.com/openjdk/jdk/blob/9ce21d1382a4f5ad601a7ee610bab64a9c575302/make/autoconf/flags-cflags.m4#L498-L511
>
> Here are a couple of discussions of that macro:
> https://stackoverflow.com/questions/7582394/strdup-or-strdup/7582741#7582741
> https://stackoverflow.com/questions/37845163/what-is-the-purpose-of-microsofts-underscore-c-functions
>
> Any suppressions of Visual Studio C4996 are probably (hopefully?) for other functions.
Thanks @kimbarrett , I had forgotten about that: https://bugs.openjdk.org/browse/JDK-6991482
-------------
PR Comment: https://git.openjdk.org/jdk/pull/18918#issuecomment-2088173007
More information about the hotspot-runtime-dev
mailing list