RFR: JDK-8266536: Provide a variant of os::iso8601_time which works with arbitrary timestamps [v3]

Thomas Stuefe stuefe at openjdk.java.net
Thu May 6 19:39:58 UTC 2021


On Thu, 6 May 2021 19:17:29 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>> 
>>  - Merge
>>  - remove harfbuzz fix
>>  - Remove duplicate coding in gtest
>>  - gtest for os.iso8601_time should use utc
>>  - Build fix for Harfbuzz
>>  - Merge
>>  - start
>
> src/hotspot/share/runtime/os.hpp line 231:
> 
>> 229:   // E.g., YYYY-MM-DDThh:mm:ss.mmm+zzzz.
>> 230:   // Returns buffer, or NULL if it failed.
>> 231:   static char* iso8601_time(jlong milliseconds_since_19700101, char buffer[29],
> 
> You are supposed to get a compiler warning on this char buffer[29]. ISO C/C++ can't pass in array type as a function argument.  As a result, the type buffer automatically casts to char* here. 
> 
> Further, the function signature is different from its implementation, which use char* buffer instead. 
> I see that you do enforce length checking in implementation.

Oh, good catch, this is a stray edit I thought I reverted. See below.

> ISO C/C++ can't pass in array type as a function argument. As a result, the type buffer automatically casts to char* here.

No, it can, that's totally fine. It gets automatically translated to pointer type, but its a valid expression nonetheless. 

It is usually done to indicate that a function expects an output array of a particular length instead of letting the caller guess the right length - and letting him to deal with truncation if he guesses wrong.

I originally planned to define the function as 

iso8601_time(jlong milliseconds_since_19700101, char buffer[29]);

to indicate that the only output value length which makes sense is 29, but then I refrained because I wanted to keep the patch simple.

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

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


More information about the hotspot-runtime-dev mailing list