RFR: JDK-8266536: Provide a variant of os::iso8601_time which works with arbitrary timestamps [v3]
Xin Liu
xliu at openjdk.java.net
Thu May 6 19:54:54 UTC 2021
On Thu, 6 May 2021 19:36:49 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> 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.
C/C++ allow you to write code like that, but they won't check the length of argument <=29 for you.
I think you sanity check inside of iso8601_time() is good enough.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3869
More information about the hotspot-runtime-dev
mailing list