RFR(S): 8145015: jni_GetStringCritical asserts for empty strings
volker.simonis at gmail.com
Mon Dec 14 08:27:48 UTC 2015
thanks for the review.
"const jchar*" means a pointer to a constant character. I removed the
'const' because that way I could get rid of the extra 'buf' variable
which was previously used to allocate and fill the temporary string.
With "const jchar * ret" it wouldn't be possible to change the
characters pointed to by 'ret'.
Returning "jachr *ret" as "const jchar * ret" on the other hand is no
problem - it is just a narrowing conversion saying that once we return
"ret" the characters it points to can not be changed any more.
So I think the code is correct.
I'd need a sponsor though, because this is a shared changed and
unfortunately we still can not push shared code changes (I'll put this
sentence in every mail now although I have little to no hope that it
will change something :(
Thank you and best regards,
On Mon, Dec 14, 2015 at 8:25 AM, David Holmes <david.holmes at oracle.com> wrote:
> +1 from me. My only query is whether:
> 3194 jchar* ret;
> should retain the const, as the return type is "const jchar *"?
> On 14/12/2015 5:11 PM, Tobias Hartmann wrote:
>> Hi Volker,
>> thanks for taking care of this! Your fix looks good to me (not a
>> On 09.12.2015 18:53, Volker Simonis wrote:
>>> can somebody please review and sponsor the following fix:
>>> The problem is that change "8141132: JEP 254: Compact Strings" removed
>>> the handling for empty strings from jni_GetStringCritical(). As a
>>> consequence, the functions will now assert with:
>>> assert(is_within_bounds(which)) failed: index 0 out of bounds 0
>>> if called with a string of length zero (because it tries to access the
>>> zeroth element of a zero-length array).
>>> The fix is trivial, just use 's_value->base(T_CHAR)' instead of
>>> While doing this fix I also noticed that jni_GetStringCritical()
>>> doesn't handle out-of-memory situations when creating a new character
>>> array (before this wasn't necessary because it always returned the
>>> original character array, but now with compressed strings it may have
>>> to allocate a new array). So I've also added the new check by using
>>> NEW_C_HEAP_ARRAY_RETURN_NULL() instead of NEW_C_HEAP_ARRAY().
>>> Notice that I also allocate one extra character for zero termination.
>>> While it seems that this is not strictly necessary, it is now the same
>>> code like in jni_GetStringChars(). And it again simplifies the
>>> handling of zero-length strings. Without the extra character it would
>>> be hard to distinguish out-of-memory errors from the allocation of
>>> zero bytes, because malloc() is free to return NULL if called with an
>>> allocation size of zero.
>>> Thank you and best regards,
More information about the hotspot-dev