svn commit: r1573794 - /subversion/trunk/subversion/libsvn_fs_fs/cached_data.c

[ko...@apache.org]

Author: kotkov
Date: Mon Mar  3 22:58:28 2014
New Revision: 1573794

URL: http://svn.apache.org/r1573794
Log:
Tweak the get_root_changes_offset() routine in order to fix the problems I
identified in http://svn.haxx.se/dev/archive-2014-02/0189.shtml
This changeset fixes the following issues in its implementation:

- It avoids seeking to a negative offset for the scenarios when the revision
  file is smaller than buffer.

- It avoids touching internal svn_stringbuf_t fields (such as DATA and LEN)
  and achieves the wanted behavior using the stringbuf API only.

- We no longer rely on the allocator-specific BLOCKSIZE when determining the
  buffer size for the trailer.  In a theoretical case where the allocator
  allocates huge 2KiB blocks for every stringbuf, we would fail to parse the
  trailer for smaller (< 2KiB) revision files.

- It uses svn_io_file_read_full2() instead of svn_io_file_read() when reading
  the file block with the revision trailer.  svn_io_file_read(), as opposed
  to svn_io_file_read_full2(), can read any number of bytes <= NBYTES before
  exiting.  This essentially means we would fail to parse the revision
  trailer in all scenarios where this function returns without filling
  the buffer.

* subversion/libsvn_fs_fs/cached_data.c
  (get_root_changes_offset): Create the stringbuf around a stack-allocated
    BUFFER.  Do that only when the buffer is filled (thus avoiding the
    necessity to touch the internal stringbuf fields, i.e. DATA, LEN and
    BLOCKSIZE).  Handle the possibility of revision files being smaller than
    the expected trailer block size (64 bytes).  Do not attempt seeking to
    negative file offsets in that case.  Finally, use svn_io_file_read_full2()
    instead of svn_io_file_read() when reading the file contents.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/cached_data.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.c?rev=1573794&r1=1573793&r2=1573794&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Mon Mar  3 22:58:28 2014
@@ -462,10 +462,13 @@ get_root_changes_offset(apr_off_t *root_
                         apr_pool_t *pool)
 {
   fs_fs_data_t *ffd = fs->fsap_data;
-  apr_off_t offset;
   apr_off_t rev_offset;
   apr_seek_where_t seek_relative;
-  svn_stringbuf_t *trailer = svn_stringbuf_create_ensure(64, pool);
+  svn_stringbuf_t *trailer;
+  char buffer[64];
+  apr_off_t start;
+  apr_off_t end;
+  apr_size_t len;
 
   /* Determine where to seek to in the file.
 
@@ -479,13 +482,13 @@ get_root_changes_offset(apr_off_t *root_
   if (svn_fs_fs__is_packed_rev(fs, rev)
       && ((rev + 1) % ffd->max_files_per_dir != 0))
     {
-      SVN_ERR(svn_fs_fs__get_packed_offset(&offset, fs, rev + 1, pool));
+      SVN_ERR(svn_fs_fs__get_packed_offset(&end, fs, rev + 1, pool));
       seek_relative = APR_SET;
     }
   else
     {
       seek_relative = APR_END;
-      offset = 0;
+      end = 0;
     }
 
   /* Offset of the revision from the start of the pack file, if applicable. */
@@ -496,16 +499,25 @@ get_root_changes_offset(apr_off_t *root_
 
   /* We will assume that the last line containing the two offsets
      will never be longer than 64 characters. */
-  SVN_ERR(svn_io_file_seek(rev_file, seek_relative, &offset, pool));
+  SVN_ERR(svn_io_file_seek(rev_file, seek_relative, &end, pool));
 
-  trailer->len = trailer->blocksize-1;
-  SVN_ERR(aligned_seek(fs, rev_file, NULL, offset - trailer->len, pool));
+  if (end < sizeof(buffer))
+    {
+      len = (apr_size_t)end;
+      start = 0;
+    }
+  else
+    {
+      len = sizeof(buffer);
+      start = end - sizeof(buffer);
+    }
 
   /* Read in this last block, from which we will identify the last line. */
-  SVN_ERR(svn_io_file_read(rev_file, trailer->data, &trailer->len, pool));
-  trailer->data[trailer->len] = 0;
+  SVN_ERR(aligned_seek(fs, rev_file, NULL, start, pool));
+  SVN_ERR(svn_io_file_read_full2(rev_file, buffer, len, NULL, NULL, pool));
 
   /* Parse the last line. */
+  trailer = svn_stringbuf_ncreate(buffer, len, pool);
   SVN_ERR(svn_fs_fs__parse_revision_trailer(root_offset,
                                             changes_offset,
                                             trailer,