[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Branko Čibej <brane_at_wandisco.com>
Date: Mon, 19 Jan 2015 02:14:39 +0100

On 19.01.2015 00:59, Stefan Fuhrmann wrote:
>
>
> On Thu, Jan 15, 2015 at 2:20 PM, Branko Čibej <brane_at_wandisco.com
> <mailto:brane_at_wandisco.com>> wrote:
>
> On 15.01.2015 13:38, ivan_at_apache.org <mailto: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.

Yes, his argument (that everything else uses plain apr_uint64_t) is valid.

-- Brane
Received on 2015-01-19 02:17:34 CET

This is an archived mail posted to the Subversion Dev mailing list.