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

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]?

I can think of several ways to do this that don't run into this bug (using a union, using inline code to get the individual bytes of FSFS_SUCCESSORS_INDEX_SIZE, or even write_full(&int), write_full(new_index + 8, sizeof(new_index)-8)). Of these three one is probably buggy, but at least one ought to be correct.

> + SVN_ERR(svn_io_file_write_full(file, new_index, sizeof(new_index), NULL,
> + pool));
> + /* No successors were created in revision zero. */
> SVN_ERR(svn_io_file_write_full(file, FSFS_SUCCESSOR_IDS_END_MARKER,
> sizeof(FSFS_SUCCESSOR_IDS_END_MARKER) - 1,
> NULL, pool));
> SVN_ERR(svn_io_file_close(file, pool));
Received on 2011-09-07 18:31:36 CEST