8218811: replace open by os::open in hotspot coding - was : open-calls in hotspot code instead of os::open ?

[Kim Barrett]

> On Feb 12, 2019, at 11:18 AM, Baesken, Matthias <matthias.baesken at sap.com> wrote:
> 
> Hello,  here is a first  webrev  + bug  :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8218811.0/
> 
> https://bugs.openjdk.java.net/browse/JDK-8218811
> 
> The  gcc    deprecation attribute for  open    in  hotspot/share/utilities/compilerWarnings_gcc.hpp   is still a bit of a hack  ,
> Maybe you have a better idea for disabling  ( is there a good way to use a pragma instead of the  define ) ?

I suggest not doing this ad hoc small piece of JDK-8214976, and
instead wait for that RFE to be addressed.  (Yeah, I know, it's
presently unassigned.  This has been coming up repeatedly recently
though, and I'm hopeful that it will get worked on soonish.)

> Additionally I was not 100 % sure - are there maybe a few places where we want to stay away from os::open  for good reason ?

Don’t know.

------------------------------------------------------------------------------
I was going to comment on some of the calls to os::open, but instead
ended up filing a couple more RFEs:

https://bugs.openjdk.java.net/browse/JDK-8218856
Improve os::open mode handling

https://bugs.openjdk.java.net/browse/JDK-8218857
Confusing overloads for os::open

------------------------------------------------------------------------------
src/hotspot/os/linux/os_linux.cpp

Why is this avoiding os::open?

------------------------------------------------------------------------------
src/hotspot/os/linux/perfMemory_linux.cpp
 101               result);;

Pre-existing extra semi-colon.

------------------------------------------------------------------------------
src/hotspot/os/linux/perfMemory_linux.cpp
 100   RESTARTABLE(os::open(destfile, O_CREAT|O_WRONLY|O_TRUNC, S_IREAD|S_IWRITE),

S_IREAD and S_IWRITE?  According to the glibc manual, "S_IREAD is an
obsolete synonym provided for BSD compatibility." and similarly for
S_IWRITE.  I think this should instead be the standard S_IRUSR|S_IWUSR.

------------------------------------------------------------------------------
src/hotspot/share/ci/ciEnv.cpp
1256     int fd = os::open(buffer, O_RDWR | O_CREAT | O_TRUNC, 0666);
...
1274     int fd = os::open(buffer, O_RDWR | O_CREAT | O_TRUNC, 0666);

Pre-existing: I think it would be better to use the named mode values
rather than a hard-coded literal.

Similarly here:
src/hotspot/share/memory/filemap.cpp
 588   int fd = os::open(_full_path, O_RDWR | O_CREAT | O_TRUNC | O_BINARY, 0444);

and here:
src/hotspot/share/utilities/ostream.cpp
 563   _fd = os::open(file_name, O_WRONLY | O_CREAT | O_TRUNC, 0666);

and here:
src/hotspot/share/utilities/vmError.cpp
1197     fd = os::open(buf, O_RDWR | O_CREAT | O_EXCL, 0666);

Given how common the 0666 pattern is, maybe a named constant in os for
that would be appropriate.

------------------------------------------------------------------------------
src/hotspot/share/utilities/compilerWarnings_gcc.hpp
  39 #ifndef DISABLE_DEPRECATE_OPEN
  40 // avoid open, use os::open
  41 extern "C" int open(const char*, int, ...) __attribute__((__deprecated__("use os::open"))); 
  42 #endif

src/hotspot/os/linux/os_linux.cpp
  26 #define DISABLE_DEPRECATE_OPEN

As mentioned above, I don't think this ought to be part of the change.
Instead, fix the open calls in os_linux.cpp too.

Also, this approach doesn't work with precompiled headers.  I suspect
the reasons for os_linux.cpp explicitly not using precompiled headers
are historical.  For example, it used to #define __STDC_FORMAT_MACROS.

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