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