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