svn commit: r1502779 - in /subversion/trunk/subversion: include/svn_io.h libsvn_subr/io.c

[st...@apache.org]

Author: stefan2
Date: Sat Jul 13 11:45:25 2013
New Revision: 1502779

URL: http://svn.apache.org/r1502779
Log:
Harden svn_io_file_aligned_seek against 0 block sizes and usage with
non-buffered files.

* subversion/include/svn_io.h
  (svn_io_file_aligned_seek): mention the buffered / non-buffered issue
                              in docstring

* subversion/libsvn_subr/io.c
  (svn_io_file_aligned_seek): fallback to APR default for block size 0;
                              handle non-buffered files gracefully

Modified:
    subversion/trunk/subversion/include/svn_io.h
    subversion/trunk/subversion/libsvn_subr/io.c

Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1502779&r1=1502778&r2=1502779&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Sat Jul 13 11:45:25 2013
@@ -2065,11 +2065,11 @@ svn_io_file_seek(apr_file_t *file,
                  apr_off_t *offset,
                  apr_pool_t *pool);
 
-/** Set the file pointer of @a file to @a offset.  In contrast to
- * #svn_io_file_seek, this function will attempt to resize the internal
- * data buffer to @a block_size bytes and to read data aligned to multiples
- * of that value.  The beginning of the block will be returned in
- * @a buffer_start, if that is not NULL.
+/** Set the file pointer of the #APR_BUFFERED @a file to @a offset.  In
+ * contrast to #svn_io_file_seek, this function will attempt to resize the
+ * internal data buffer to @a block_size bytes and to read data aligned to
+ * multiples of that value.  The beginning of the block will be returned
+ * in @a buffer_start, if that is not NULL.
  * Uses @a pool for temporary allocations.
  *
  * @note Due to limitations of the APR API, in particular pre-1.3 APR,
@@ -2077,6 +2077,8 @@ svn_io_file_seek(apr_file_t *file,
  * function on @a file, you are, however, virtually guaranteed to get at
  * least 4kByte alignments for all reads.
  *
+ * @note Calling this for non-buffered files is legal but inefficient.
+ *
  * @since New in 1.9
  */
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1502779&r1=1502778&r2=1502779&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sat Jul 13 11:45:25 2013
@@ -3564,8 +3564,9 @@ svn_io_file_aligned_seek(apr_file_t *fil
                          apr_off_t offset,
                          apr_pool_t *pool)
 {
-  apr_size_t file_buffer_size = 4096;
-  apr_off_t desired_buffer = 0;
+  const apr_size_t apr_default_buffer_size = 4096;
+  apr_size_t file_buffer_size = apr_default_buffer_size;
+  apr_off_t desired_offset = 0;
   apr_off_t current = 0;
   apr_off_t aligned_offset = 0;
   svn_boolean_t fill_buffer = FALSE;
@@ -3573,10 +3574,20 @@ svn_io_file_aligned_seek(apr_file_t *fil
   /* paranoia check: huge blocks on 32 bit machines may cause overflows */
   SVN_ERR_ASSERT(block_size == (apr_size_t)block_size);
 
+  /* default for invalid block sizes */
+  if (block_size == 0)
+    block_size = apr_default_buffer_size;
+
   /* on old APRs, we are simply stuck with 4k blocks */
 #if APR_VERSION_AT_LEAST(1,3,0)
   file_buffer_size = apr_file_buffer_size_get(file);
-  if (file_buffer_size != (apr_size_t)block_size)
+
+  /* don't try to set a buffer size for non-buffered files! */
+  if (file_buffer_size == 0)
+    {
+      aligned_offset = offset;
+    }
+  else if (file_buffer_size != (apr_size_t)block_size)
     {
       /* FILE has the wrong buffer size. correct it */
       char *buffer;
@@ -3588,7 +3599,6 @@ svn_io_file_aligned_seek(apr_file_t *fil
       aligned_offset = offset - (offset % block_size);
       fill_buffer = TRUE;
     }
-  else
 #endif
     {
       aligned_offset = offset - (offset % file_buffer_size);
@@ -3627,9 +3637,9 @@ svn_io_file_aligned_seek(apr_file_t *fil
     }
 
   /* finally, seek to the OFFSET the caller wants */
-  desired_buffer = offset;
+  desired_offset = offset;
   SVN_ERR(svn_io_file_seek(file, SEEK_SET, &offset, pool));
-  if (desired_buffer != offset)
+  if (desired_offset != offset)
     return do_io_file_wrapper_cleanup(file, APR_EOF,
                                       N_("Can't seek in file '%s'"),
                                       N_("Can't seek in stream"),