On 15 January 2015 at 16:20, Branko Čibej <br...@wandisco.com> wrote: > On 15.01.2015 13:38, i...@apache.org wrote: >> Author: ivan >> Date: Thu Jan 15 12:38:13 2015 >> New Revision: 1652076 >> >> URL: http://svn.apache.org/r1652076 >> Log: >> Fix mostly theoretical data corruption in log-addressing enabled FSFS >> repositories when physical-to-logical index will be more than 4 GB on >> 32-bit platform. >> >> * subversion/libsvn_fs_fs/index.c >> (svn_fs_fs__p2l_index_append): Use proper type (apr_uint64_t) for local >> variable that holds file size. > > How about using svn_filesize_t instead, for clarity. Or apr_off_t which > should always have the required range. > I agree that svn_filesize_t is more appropriate type, but most of FSFS log-addressing code in index.c uses apr_uint64_t. For example svn_fs_fs__p2l_index_append: [[[ apr_uint64_t last_entry_end = 0; apr_uint64_t last_page_end = 0; apr_uint64_t last_buffer_size = 0; /* byte offset in the spill buffer at the begin of the current revision */ apr_uint64_t file_size = 0; ]]]
Packed stream functions (stream_write_encoded() etc) also accepts apr_uint64_t. So I decided just fix obvious mistake and leave decision on proper type to people with more knowledge how index.c supposed to work. Btw I get several warning when compiling index.c in Windows 32-bit (there are unrelated to r1652076): [[[ ..\..\..\subversion\libsvn_fs_fs\index.c(1356): warning C4389: '!=' : signed/unsigned mismatch ..\..\..\subversion\libsvn_fs_fs\index.c(1738): warning C4018: '>=' : signed/unsigned mismatch ..\..\..\subversion\libsvn_fs_fs\index.c(2496): warning C4018: '<' : signed/unsigned mismatch ..\..\..\subversion\libsvn_fs_fs\index.c(2769): warning C4018: '>=' : signed/unsigned mismatch ..\..\..\subversion\libsvn_fs_fs\index.c(2796): warning C4018: '>' : signed/unsigned mismatch ]]] -- Ivan Zhakov
