RFR (XS) : 8141641: Runtime: implement range for ErrorLogTimeout

[gerard ziemski]

Thank you David and Dmitry for reviews. Please see my response in-line:


>> 3) Also, David suggested to wrap constants in multiplication in
>> src/share/vm/runtime/thread.cpp into CONST64 to avoid possible
>> overflows, i.e.:
>> os::sleep(this, ErrorLogTimeout * CONST64(1000), false);

I must have missed something - where/when did David suggest doing this?


> I'm making a bit of a hash of this review - sorry.
>
> I agree with Dmitry that the os::sleep call should be changed as above:
>
> os::sleep(this, ErrorLogTimeout * CONST64(1000), false);
>
> but as the sleep arg is a jlong we need to limit ErrorLogTimeout to (MAX_JLONG/1000) _but_ that's too big for uintx on
> 32-bit. Which means the true range is different on 32-bit versus 64-bit. So if I finally get this right we should have
> the range as:
>
> range(0, LP64_ONLY(MAX_JLONG/1000) NOT_LP64(UINTX_MAX))
>
> Not sure we have MAX_JLONG defined as a constant. I don't agree with just selecting the 32-bit range as that again
> becomes an arbitrary constraint on 64-bit.

Since os:sleep() takes jlong (8 bytes long on both 32 and 64 bit), them it would appear that the most straightforward 
solution is to change ErrorLogTimeout type from "uintx" to "uint64_t" (also 8 bytes on both 32 and 64 bit) and then we 
can have the range simply defined as:

range(0, max_jlong/1000)

And then we can have:

os::sleep(this, ErrorLogTimeout * CONST64(1000), false); // in seconds

Do we agree?


cheers