On Thu, 11 Sep 2025 17:50:10 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/security/util/Password.java line 193: >> >>> 191: ba[bb.position()] = '\n'; >>> 192: } >>> 193: return ba; >> >> Some random comments. This is all internal to this class, and it's all >> vaguely cleanup, so it's up to you whether to take any action on any of >> these points. >> >> It might be worth having a comment on this method that explains why it's >> doing this. The "obvious" way of encoding the characters is to use >> String.getBytes(), but you probably don't want to create a String, because >> (I think) you want to be able to erase the arrays afterwards. Worth >> explaining, if true. >> >> At lines 190-191 the mixture of buffer and array stuff is kind of confusing. >> I think you can check whether bb.remaining() > 0 and use bb.put(). >> >> There are actually three possibilities for what's in the returned byte >> array: 1) it's completely filled with encoded bytes from the input; 2) it >> contains the encoded input followed by an NL byte; or 3) it contains the >> encoded input, an NL byte, followed by an arbitrary number of zero bytes. >> The way that the returned array is processed by the caller handles this, but >> it seems a bit brittle. One possibility is always to return an array of the >> exact correct length, including an NL byte. Of course this entails an extra >> copy (and an extra array to erase) but there are fewer cases for the caller >> to handle. > > Thanks. I'll add some comments and do not mix buffer and array. The `\n` > trick as a stop sign will be kept to avoid too many changes. Thanks for the updates. Re-approving in case it's necessary. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27196#discussion_r2342858920
