RFR: 8214944: replace strerror by os::strerror

[Baesken, Matthias]

Hi David,  here is a little   example program  showing   the  strerror-related warning   of VS2017 
(guess it works also with lower VS versions) ;  compile  with -W3   :

$ cl strwarn.cpp -W3 -o strwarn
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24234.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.
 ......
strwarn.cpp(12): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
  ....


    -------------------
/*  see : jdk/src/hotspot/share/utilities/globalDefinitions_visCPP.hpp
#pragma warning( disable : 4996 ) // unsafe string functions. Same as define _CRT_SECURE_NO_WARNINGS/_CRT_SECURE_NO_DEPRICATE
*/

#include <errno.h>
#include <stdio.h>
#include <string.h>

int main() {
  FILE* f = fopen ("nofile","r");
  if (f == NULL) {
    printf ("Error opening file: %s \n",strerror(errno));
  }
  return 0;
}
   -------------------------

However we disable   warning C4996    in    jdk/src/hotspot/share/utilities/globalDefinitions_visCPP.hpp   , 
so no wonder we do not get it.
Probably  removing the  disabling  of    warning C4996    in    jdk/src/hotspot/share/utilities/globalDefinitions_visCPP.hpp      might  cause  much more warnings than just  the strerror-warning  "family" ,  
so I am not sure if it is really  an option  at the moment .


Best regards, Matthias



> -----Original Message-----
> From: Baesken, Matthias
> Sent: Donnerstag, 6. Dezember 2018 14:20
> To: 'David Holmes' <david.holmes at oracle.com>; 'hotspot-
> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> Cc: 'magnus.ihse.bursie at oracle.com' <magnus.ihse.bursie at oracle.com>
> Subject: RE: RFR: 8214944: replace strerror by os::strerror
> 
> Hi  David and Nils,  thanks for  the reviews .
> 
> >
> > Pity there is no way to detect when we use something like that which we
> > shouldn't.
> >
> 
> Seems VS2017   has compiler warnings for this , at least
> 
> https://github.com/fmtlib/fmt/issues/703
> 
> suggests this .
> See the example   output   :
> 
> 
> format.cc:172:17: warning: 'strerror' is deprecated: This function or variable
> may be unsafe. Consider using strerror_s instead. To disable deprecation,
> use _CRT_SECURE_NO_WARNINGS. See online help for details. [-
> Wdeprecated-declarations]
>       buffer_ = strerror(error_code_);
>                 ^
> C:\Program Files (x86)\Windows
> Kits\10\Include\10.0.16299.0\ucrt\string.h:180:16: note: 'strerror' has been
> explicitly marked deprecated here
> _Check_return_ _CRT_INSECURE_DEPRECATE(strerror_s)
>                ^
> 
> Might be we do not have these warnings enabled for some reason .
> 
> 
> Best regards, Matthias
> 
> 
> > -----Original Message-----
> > From: David Holmes <david.holmes at oracle.com>
> > Sent: Donnerstag, 6. Dezember 2018 13:57
> > To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> > dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> > Subject: Re: RFR: 8214944: replace strerror by os::strerror
> >
> > Hi Matthias,
> >
> > On 6/12/2018 8:36 pm, Baesken, Matthias wrote:
> > > Hello  , please review this small fix .
> > >
> > > Hotspot still has a few uses of the thread unsafe strerror function.
> > > Most places where strerror has been used previously, were already
> > replaced by os::strerror  (using a errno_to_string error text array).
> > >
> > > But a few places remained and should also use better use  os:strerror.
> >
> > It's not that they "remain" but were added later. Previous strerror
> > replacement was done under JDK-8148425. The new uses in OS-specific
> > get_mtime functions were added in a few weeks later by JDK-8146879. The
> > ZGC use is obviously much more recent.
> >
> > Pity there is no way to detect when we use something like that which we
> > shouldn't.
> >
> > Fix looks good.
> >
> > Thanks,
> > David
> > -----
> >
> > >
> > > Bug/webrev :
> > >
> > > https://bugs.openjdk.java.net/browse/JDK-8214944
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8214944.0/
> > >
> > >
> > > Thanks, Matthias
> > >