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