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

[Baesken, Matthias]

Thanks for the feedback .
Do you see any  special  issue with the os::open implementations that forbid usage in os::error  ?
Kim what do you think ?

I found  over  80  references to os::…  calls   in   vmError.cpp  so I think it is not generally a bad thing to do it  (however for course there might be functions in the os:: layer that
  cause trouble in error reporting).


New webrev :  (compilerWarnings_gcc.hpp   deprecation open  deprecation   + vmError.cpp  adjustment removed)


http://cr.openjdk.java.net/~mbaesken/webrevs/8218811.1/


Best regards, Matthias


From: Thomas Stüfe <thomas.stuefe at gmail.com>
Sent: Mittwoch, 13. Februar 2019 10:41
To: Baesken, Matthias <matthias.baesken at sap.com>
Cc: hotspot-dev at openjdk.java.net; Kim Barrett <kim.barrett at oracle.com>
Subject: Re: 8218811: replace open by os::open in hotspot coding - was : open-calls in hotspot code instead of os::open ?

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<mailto: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<mailto:hotspot-dev at openjdk.java.net>; 'Kim Barrett' <kim.barrett at oracle.com<mailto: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<mailto:kim.barrett at oracle.com>>
> > To: Ioi Lam <ioi.lam at oracle.com<mailto:ioi.lam at oracle.com>>
> > Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net<mailto: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<mailto: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<mailto: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
> >
> >