[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Rework overflow check when reading big-endian 64bit ints

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Tue, 13 Sep 2011 03:27:07 +0300

On the fs-successor-ids branch we store 64bit ints in big endian format
in one of the files. The following patch changes the function which
reads that file such that it returns an apr_off_t offset (which is
convenient for further processing), and in return moves and changes the
overflow check.

I'll fix the overflow check which I add to mirror the wording of the one
which I remove. Other than that, does below look correct?

Thanks.

[[[
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1169978)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -5950,7 +5950,7 @@ copy_file_partially(apr_file_t *source_file,
  * handle, which is allocated in RESULT_POOL.
  */
 static svn_error_t *
-read_successor_index_entry(apr_uint64_t *revision_offset,
+read_successor_index_entry(apr_off_t *revision_offset,
                            apr_file_t **file_p,
                            svn_fs_t *fs,
                            svn_revnum_t revision,
@@ -5962,6 +5962,7 @@ static svn_error_t *
   apr_file_t *file;
   apr_uint32_t m;
   apr_uint32_t n;
+ apr_uint64_t nm;
   const char *local_abspath = path_successor_ids(fs, revision, scratch_pool);
 
   /* The trivial case: The caller asked for the first revision in a file. */
@@ -6002,7 +6003,13 @@ static svn_error_t *
                                _("Missing data at offset %llu in file '%s'"),
                                offset + 4, local_abspath);
     }
- *revision_offset = ((apr_uint64_t)(ntohl(n)) << 32) | ntohl(m);
+ nm = ((apr_uint64_t)(ntohl(n)) << 32) | ntohl(m);
+ if ((apr_off_t)nm != nm)
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ _("Successors data lives at offset larger than "
+ "this platform can represent (%ld<<32 + %ld)"),
+ (long)n, (long)m);
+ *revision_offset = nm;
 
   if (file_p && ! *file_p)
     *file_p = file;
@@ -6044,23 +6051,12 @@ update_successor_ids_file(const char **successor_i
     }
   else
     {
- apr_uint64_t prev_successor_ids_offset;
+ apr_off_t prev_successor_ids_offset;
       apr_file_t *successor_ids_file;
 
       /* Figure out the offset of successor data for the previous revision. */
       SVN_ERR(read_successor_index_entry(&prev_successor_ids_offset,
                                          NULL, fs, new_rev - 1, pool, pool));
-
- /* Check for offset overflow.
- * This gives a "will never be executed" warning on some platforms. */
- if (sizeof(apr_off_t) < sizeof(apr_uint64_t) &&
- prev_successor_ids_offset > APR_UINT32_MAX)
- return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
- _("Cannot seek to offset %llu in successor "
- "data file; platform does not support "
- "large files; this repository can only "
- "be used on platforms which support "
- "large files"), prev_successor_ids_offset);
 
       /* Copy successor IDs index and the data for revisions older than
        * the previous revision into the temporary successor data file. */
]]]
Received on 2011-09-13 02:28:07 CEST

This is an archived mail posted to the Subversion Dev mailing list.