On Wed, Sep 07, 2011 at 07:31:05PM +0300, Daniel Shahaf wrote: > On Wednesday, September 07, 2011 4:16 PM, s...@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.
