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

[Thomas Stüfe]

Hi Matthias,

not a full review but please do not use os::open in error reporting:

http://cr.openjdk.java.net/~mbaesken/webrevs/8218811.0/src/hotspot/share/utilities/vmError.cpp.udiff.html

We should use as little as possible VM functions here, since they may be
broken and/or pull other VM dependencies which may be broken or introduce
circularities.

I am not sure if the same argument holds for other call sites as well.

Best Regards, Thomas



On Tue, Feb 12, 2019 at 5:19 PM 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 ) ?
>
> Additionally I was not 100 % sure - are there maybe a few places where we
> want to stay away from os::open  for good reason ?
>
>
> Best regards, Matthias
>
>
> > -----Original Message-----
> > From: Baesken, Matthias
> > Sent: Dienstag, 12. Februar 2019 10:17
> > To: hotspot-dev at openjdk.java.net; 'Kim Barrett' <kim.barrett at oracle.com>
> > Subject: 8218811: replace open by os::open in hotspot coding - was :
> open-
> > calls in hotspot code instead of os::open ?
> >
> > Hi  Ioi / Kim  I created
> >
> > https://bugs.openjdk.java.net/browse/JDK-8218811
> >
> > 8218811:  replace open by os::open in hotspot coding
> >
> > >
> > > Not yet, but see https://bugs.openjdk.java.net/browse/JDK-8214976
> > >
> >
> > Regarding     https://bugs.openjdk.java.net/browse/JDK-8214976
> >
> >
> > > For functions which should never be called outside the implementation
> of
> > the os replacement, we can use (for example)
> > >
> > > extern "C" int vsnprintf(char*, size_t, const char*, va_list)
> > > __attribute__((__deprecated__("use os::vsnprintf")));
> > >
> > > and in the definition of os::vsnprintf, locally disable the deprecation
> > warning with the appropriate diagnostic #pragma.
> >
> > Should I add something like this for open   to  compilerWarnings.hpp  ?
> > I think if yes, I better restrict this for now to gcc  .
> >
> >
> > Best regards, Matthias
> >
> > >
> > > Message: 4
> > > Date: Thu, 7 Feb 2019 12:40:00 -0500
> > > From: Kim Barrett <kim.barrett at oracle.com>
> > > To: Ioi Lam <ioi.lam at oracle.com>
> > > Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net>
> > > Subject: Re: open-calls in hotspot code instead of os::open ?
> > > Message-ID: <0FD37FD4-A478-4849-B474-A3A8CDCDD6D5 at oracle.com>
> > > Content-Type: text/plain; charset=us-ascii
> > >
> > > > On Feb 6, 2019, at 9:08 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
> > > >
> > > > I think this should be fixed.
> > >
> > > +1
> > >
> > > > BTW, is there a way to forbid all the calls to ::open()?
> > >
> > > Not yet, but see https://bugs.openjdk.java.net/browse/JDK-8214976
> > >
> > >
>
>