On 19.01.2015 00:59, Stefan Fuhrmann wrote: > > > On Thu, Jan 15, 2015 at 2:20 PM, Branko Čibej <br...@wandisco.com > <mailto:br...@wandisco.com>> wrote: > > On 15.01.2015 13:38, i...@apache.org <mailto: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. > > > Any of these would be fine(-ish). With apr_off_t, compilers > would require an explicit downcast because the svn_filesize_t > returned by the spill buffer is always an int64. The values are > guaranteed to be non-negative, which makes conversion to > unsigned safe. > > The code in question is part of the "constructor" that writes > the index section of some file where the target type is uint64. > Limitations of the local system will be checked when the index > gets read again. > > So ultimately, we will convert to uint64 before writing to disk. > Ivan chose to do it immediately and there is no drawback to that.
Yes, his argument (that everything else uses plain apr_uint64_t) is valid. -- Brane
