RFR: 8214976: Warn about uses of functions replaced for portability [v2]

Harold Seigel hseigel at openjdk.java.net
Tue Feb 1 14:13:57 UTC 2022


On Fri, 28 Jan 2022 04:55:48 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   changes to address some review comments
>
> src/hotspot/share/runtime/os.hpp line 533:
> 
>> 531:   // platforms that support such things.  This calls shutdown() and then aborts.
>> 532:   static void abort(bool dump_core, void *siginfo, const void *context);
>> 533:   static void abort(bool dump_core);
> 
> I don't understand why the change to the default arg was needed. There should be no conflict between `os::abort()` and `::abort()`.

I reverted the abort() changes.  Thanks for correcting this.

> src/hotspot/share/utilities/compilerWarnings_gcc.hpp line 97:
> 
>> 95: FORBID_C_FUNCTION(FILE*    fopen(const char*, const char*),           "use os::fopen");
>> 96: FORBID_C_FUNCTION(int      fsync(int),                                "use os::fsync");
>> 97: FORBID_C_FUNCTION(int      ftruncate(int, off_t),                     "use os::ftruncate");
> 
> Shouldn't this be ftruncate for BSD and ftruncate64 for other Posix (not sure what Windows has)?

Platform agnostic code would call ftruncate(), not ftruncate64().  So I think this is correct as is.

> src/hotspot/share/utilities/compilerWarnings_gcc.hpp line 99:
> 
>> 97: FORBID_C_FUNCTION(int      ftruncate(int, off_t),                     "use os::ftruncate");
>> 98: FORBID_C_FUNCTION(void     funlockfile(FILE *),                       "use os::funlockfile");
>> 99: FORBID_C_FUNCTION(off_t    lseek(int, off_t, int),                    "use os::lseek");
> 
> Similarly there should be a lseek64 definition too.

Like ftruncate(), platform agnostic code would call lseek(), not lseek64().  So I think this is correct as is.

> src/hotspot/share/utilities/ostream.cpp line 615:
> 
>> 613: 
>> 614: PRAGMA_DIAG_PUSH
>> 615: PRAGMA_PERMIT_FORBIDDEN_C_FUNCTION(write);
> 
> Why do we not call os::write here?

fixed

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

PR: https://git.openjdk.java.net/jdk/pull/7248


More information about the hotspot-dev mailing list