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

Re: svn commit: r1166247 - in /subversion/branches/fs-successor-ids/subversion/libsvn_fs_fs: fs.h fs_fs.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Wed, 7 Sep 2011 18:43:01 +0200

On Wed, Sep 07, 2011 at 07:31:05PM +0300, Daniel Shahaf wrote:
> On Wednesday, September 07, 2011 4:16 PM, stsp_at_apache.org wrote:
> > + /* Create successor-ID data for revision zero. */
> > SVN_ERR(svn_io_file_open(&file, path_successor_ids(fs, 0, pool),
> > APR_WRITE | APR_BUFFERED | APR_CREATE,
> > APR_OS_DEFAULT, pool));
> > + /* Write the new index. */
> > + memset(new_index, 0, sizeof(new_index));
> > + n = (apr_int32_t*)&new_index[3];
> > + *n = htonl(FSFS_SUCCESSORS_INDEX_SIZE);
>
> That doesn't look right. You're assuming this trick sets new_index[0..3], but can't it set new_index[3..7]?

This code must set new_index[3..7].

Recall that the on-disk format of an offset is essentially two big-endian
32-bit integers, with the most significant integer coming first.
I.e. we want the initial index to look like this:
 [ 4 bytes zero | htonl(FSFS_SUCCESSORS_INDEX_SIZE) | zero ... ]

When reading the offset for revision zero, the code reads the first
4 bytes into variable 'n', and the second 4 bytes into variable 'm'.
It then combines both into a 64bit offset in native endianness:
  *revision_offset = ((apr_uint64_t)(ntohl(n)) << 32) | ntohl(m);

Note that due to a shortcut in the code reading the index we never
actually read the offset for revision 0 from disk (it is always equal
to FSFS_SUCCESSORS_INDEX_SIZE, after all). But the initial index we
write should be correct regardless.
Received on 2011-09-07 18:43:39 CEST

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