RFR: 8296158: Refactor the verification of CDS region checksum
Matias Saavedra Silva
matsaave at openjdk.org
Tue Feb 14 21:31:43 UTC 2023
On Sat, 4 Feb 2023 01:22:16 GMT, Ioi Lam <iklam at openjdk.org> wrote:
> Consolidate the two old functions
>
> - `FileMapInfo::region_crc_check(char* buf, size_t size, int expected_crc)`
> - `FileMapInfo::verify_region_checksum(int i)`
>
> Into a single function `FileMapRegion::check_region_crc()`.
>
> Also performed some minor renaming and removed a useless API.
Changes requested by matsaave (Author).
src/hotspot/share/cds/filemap.cpp line 1599:
> 1597: // loaded into memory via FileMapInfo::map_region() or FileMapInfo::read_region().
> 1598: // I.e., this->mapped_base() must be valid.
> 1599: if (!VerifySharedSpaces) {
While I think this is a nice shortcut to avoid checking VerifySharedSpaces, I think it occludes the fact it is a necessary condition in the if statements below. Just my 2 cents but I think it is more intuitive to keep `if (VerifySharesSpaces && !check_region_crc())` and keep an assert inside this method.
Either way does work, so go with what you think makes the most sense. Other than that, LGTM!
-------------
PR: https://git.openjdk.org/jdk/pull/12424
More information about the hotspot-runtime-dev
mailing list