On 13.01.2015 19:59, Ben Reser wrote: > On 10/13/14 3:54 PM, stef...@apache.org wrote: >> Author: stefan2 >> Date: Mon Oct 13 22:54:13 2014 >> New Revision: 1631598 >> >> URL: http://svn.apache.org/r1631598 >> Log: >> Add FSFS index checksum verification code to 'svnadmin verify'. >> >> We don't verify the index data against the checksums on every >> access as there is plenty of cross-verification within the indexes, >> between them as well as between index and rev data. Only if some >> inconsistency has been detected and the user wants to trace down >> the source (using 'svnadmin verify'), will we verify that the index >> data has not been tampered with. >> >> * subversion/libsvn_fs_fs/verify.c >> (verify_index_checksum, >> verify_index_checksums): New per rev / pack file verification code. >> (verify_index_consistency): Call the new index checksum test before >> using any of the index contents. >> >> * subversion/tests/libsvn_fs_fs/fs-fs-fuzzy-test.c >> (fuzzing_1_byte_1_rev): Remove the code that would accept certain >> index modifications. Accept only case changes >> in index MD5 digests because they don't affect >> the validity. > [snip] > >> @@ -144,14 +120,30 @@ fuzzing_1_byte_1_rev(const char *repo_na >> err = svn_repos_verify_fs3(repos, revision, revision, TRUE, FALSE, >> FALSE, >> NULL, NULL, NULL, NULL, iterpool); >> >> - /* Benign changes may or may not be detected. */ >> - if (!is_legal) >> + /* Case-only changes in checksum digests are not an error. >> + * We allow upper case chars to be used in MD5 checksums in all other >> + * places, thus restricting them here would be inconsistent. */ >> + if ( i >= filesize - footer_len /* Within footer */ >> + && c_old >= 'a' && c_old <= 'f' /* 'a' to 'f', only appear >> + in checksum digests */ >> + && c_new == c_old - 'a' + 'A') /* respective upper case */ >> + { >> + if (err) >> + { >> + /* Let us know where we were too strict ... */ >> + printf("Detected case change in checksum digest at offset 0x%" >> + APR_UINT64_T_HEX_FMT " (%" APR_OFF_T_FMT ") in r%ld: " >> + "%c -> %c\n", i, i, revision, c_old, c_new); >> + >> + SVN_ERR(err); >> + } >> + } >> + else if (!err) >> { >> /* Let us know where we miss changes ... */ >> - if (!err) >> - printf("Undetected mod at offset %"APR_UINT64_T_HEX_FMT >> - " (%"APR_OFF_T_FMT") in r%ld: 0x%02x -> 0x%02x\n", >> - i, i, revision, c_old, c_new); >> + printf("Undetected mod at offset 0x%"APR_UINT64_T_HEX_FMT >> + " (%"APR_OFF_T_FMT") in r%ld: 0x%02x -> 0x%02x\n", >> + i, i, revision, c_old, c_new); >> >> SVN_TEST_ASSERT(err); >> } >> >> > Those printf formats aren't valid on all platforms/builds of APR. In this > case > i is an apr_off_t and you're using APR_UINT64_T_HEX_FMT with it. > Unfortunately > you can't guarantee that apr_off_t is a 64-bit int. If LFS support isn't > enabled and the platform has its own off_t then APR will use whatever the size > is for that, or just an int if off_t isn't defined. See aprenv.py in APR's > source code for the various scenarios. > > Not sure what you want to do about this since it makes the hex output more > difficult to do. I'd probably just remove the hex output since it's a test > and > we can probably deal with that bit of convenience.
Since it is a test, what's wrong with just casting the first vararg to (apr_uint64_t) instead, since we "know" (i.e., hope) that off_t won't overflow 64 bits ... -- Brane
