On Wed, 24 Sep 2025 15:50:21 GMT, Mark Powers <[email protected]> wrote:
>> src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line
>> 1487:
>>
>>> 1485: int writeIterationCount = macIterationCount;
>>> 1486:
>>> 1487: if (newKeystore) {
>>
>> I cannot see how `newKeystore` is useful. Back when it's set to true,
>> `macAlgorithm` was also set to `defaultMacAlgorithm()`. Therefore there is
>> no need to try `defaultMacAlgorithm()` again below. As for
>> `writeIterationCount`, for a new keystore it is -1, so we can use this to
>> decide whether it should be assigned `defaultMacIterationCount()`.
>
> `newKeystore` and `macAlgorithm` are not always set together. When creating a
> keystore, they are both set, but when reading a keystore only `macAlgorithm`
> is set. So if I read first and then write (no create), `newKeystore` will not
> be set. Therefore, I can't remove `newKeystore` and only use `macAlgorithm`
> if that's what you're suggesting.
>
> I agree that `defaultMacAlgorithm()` can be replaced by `macAlgorithm` on
> lines 1489 and 1490.
>
> `writeIterationCount` is initialized to `defaultMacIterationCount()` on. line
> 1253 so it is never -1 when `calculateMac` is entered. `writeIterationCount`
> is also set when a keystore is read (lines 2209 and 2220). I probably
> shouldn't be doing that.
Maybe `macAlgorithm` shouldn't be set when reading a keystore.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2376288340
