Re: svn commit: r1652076 - /subversion/trunk/subversion/libsvn_fs_fs/index.c

On Thu, Jan 15, 2015 at 2:20 PM, Branko Čibej <brane_at_wandisco.com> wrote:

> On 15.01.2015 13:38, ivan_at_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.

-- Stefan^2.
Received on 2015-01-19 01:00:30 CET