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

[Thomas Stuefe]

On Thu, 29 Aug 2024 07:59:47 GMT, David Holmes <dholmes at openjdk.org> wrote:

> > Many of these translations seem awkward, since they convert to size_t only to then convert back to int.
> 
> Can you be more specific here please? 

Certainly. For example, this construct:


    size_t utf8_len = static_cast<size_t>(length);
    const char* base = UNICODE::as_utf8(position, utf8_len);
    Symbol* sym = SymbolTable::new_symbol(base, checked_cast<int>(utf8_len));


We introduce `utf8_len` as a `size_t` synonym for len, but since it originates from an `int`, its length must be <= INT_MIN. We also assume it is >=0, but we don't check. We feed `utf8_len` into both `UNICODE::as_utf8` and `SymbolTable::new_symbol`. The former takes a size_t, but since we rely on `length` >= 0, we could just as well give it `length`. For `SymbolTable::new_symbol`, we translate `utf8_len` back to `int`, with a check. The check feels superfluous since `utf8_len` came from an int.

I assume this verbosity is for the benefit of the code reader, to make intent clear. Otherwise, we could have just continued to use length, and just cast it on the fly to unsigned or to size_t when calling `UNICODE::as_utf8`.

My thought was that if `SymbolTable::new_symbol` would take size_t too, the introduction of utf8_len would serve more of a point.

> I know the most awkward case is where we have an in/out parameter that brings in an int and sends out a size_t, but there is not much to do about that without converting everything to int and I think there will be a lot of fan out because ultimately these mostly come back to array lengths, and symbol lengths, which are all int (and don't need to be bigger).
> 
> So I would really prefer to not try and apply more bandages at this stage. I really just want this part fixed so I can proceed with the JNI update.

Okay, sure. I'll review the patch for correctness then in its current form.

> 
> Thanks.

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

PR Comment: https://git.openjdk.org/jdk/pull/20560#issuecomment-2317033703