On Fri, 5 Sep 2025 09:29:28 GMT, Mikhail Yankelevich <[email protected]> wrote:
>> I may be misunderstanding your comment but it looks like you're using a >> single, `double` value to compare versions. The original code did that but >> it doesn't work in all cases. The problem occurs when checking a range such >> as >> >> `version >= 3.11 && version < 3.12` >> >> If the version number is 3.111, then the comparison is `true` and tests are >> skipped, even though _version_ 3.111 is "greater" than 3.12. So this code >> creates two doubles: `major` and `minor` to do separate comparisons of the >> values. >> >> To further complicate things, NSS also has versions of the form `x.y.z`. The >> original code combined the 'y' and 'z' components to go from 3.11.9 to >> 3.119. My change would create major version 3.0 and minor version 11.9. > > Ah, you're right, I didn't think of this case! This approach will be easier > then, I agree. > > Still, `ver.substring(idx+1)` may cause errors in cases like this [NSS > 3.15.3.1 release > notes](https://nss-crypto.org/reference/security/nss/legacy/nss_releases/nss_3.15.3.1_release_notes/index.html#mozilla-projects-nss-nss-3-15-3-1-release-notes). > It will try to convert 15.3.1 to a double. Do you think changing to > something like this would be worth it? > > > String verSubstring = ver.substring(idx+1); > String[] splitParts = verSubstring.split("//."); > Double minor = Double.parseDouble(splitParts.length > 1 > ? splitParts[0] + "." + splitParts[1] > : splitParts[0]); That's a good point about 4-component version numbers. Frankly, I was hoping that NSS never did that. I refactored the version parsing code to create a `Version` record with major, minor, and patch versions. This way, individual tests don't have to do the String parsing. If we end up with a test that has to be skipped due to that 4th component, we can update the parsing accordingly. Alternately, I can add it since I'm here now... what is that fourth component called? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27095#discussion_r2325142358
