RFR: 8338257: UTF8 lengths should be size_t not int [v7]

David Holmes dholmes at openjdk.org
Thu Aug 29 12:33:25 UTC 2024


On Thu, 29 Aug 2024 08:52:03 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> David Holmes 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 13 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8338257-utf8-length
>>  - Extra assertion requested by tstuefe
>>  - more missing casts
>>  - fix cast
>>  - missing cast
>>  - Fix incorrect comments and size_t use per Dean's review
>>  - Add missing cast for signed-to-unsigned converion.
>>  - unnecessary cast
>>  - Fix comments
>>  - Fix off-by-one error
>>  - ... and 3 more: https://git.openjdk.org/jdk/compare/e81bf3da...9dce4ffb
>
> src/hotspot/share/classfile/javaClasses.hpp line 138:
> 
>> 136:   // Legacy variants that truncate the length if needed
>> 137:   static int    utf8_length_as_int(oop java_string);
>> 138:   static int    utf8_length_as_int(oop java_string, typeArrayOop string_value);
> 
> I don't get the point of this variant of the function. It takes a string and a typearray. What is the contract here, is the only value allowed for typearray the array oop underlying the string? If yes, why do you assert for value equality in the function? That implies I can feed in any typearrayoop here as long as it has the same value as the string.
> 
> I can only see a single point where this function is used, so that does not explain much. Maybe I am overlooking something, but why not just inline the code into that one using call site?

The as_int versions mirror the existing not-as-int versions. I admit I don't really see the point of unwrapping the array from the string but then pass them both. I assume they are intended/required to be a matching pair.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20560#discussion_r1736113281


More information about the hotspot-dev mailing list