Hi,
I was reviewing r1931334 nomination and noticed that svn__adler32() uses
apr_off_t for buffer size. The apr_off_t stands for an offset in a file. It
can be larger or even smaller than maximum addressable size (apr_size_t).
I have prepared patch that fixes this. Patch changes the diff code which
very complicated, so review will be much appreciated.
Log message:
[[[
Use apr_size_t instead of apr_off_t for buffer size in svn__adler32().
The apr_off_t stands for an offset in a file. It can be larger or even
smaller
than maximum addressable size (apr_size_t).
Update all callers.
* subversion/include/private/svn_adler32.h
* subversion/libsvn_subr/adler32.c
(svn__adler32): Use apr_size_t instead of apr_off_t for SIZE argument.
* subversion/libsvn_diff/diff.h
* subversion/libsvn_diff/util.c
(svn_diff__normalize_buffer): Use apr_size_t instead of apr_off_t for
LENGTGP argument.
* subversion/libsvn_diff/diff_file.c
(datasource_get_next_token): Use apr_size_t instead of apr_off_t for
local variable LENGTH.
(token_compare): Rewrite a code a bit to make it clear that size is
apr_size_t.
* subversion/libsvn_diff/diff_memory.c
(datasource_get_next_token, token_compare): Use apr_size_t instead of
apr_off_t for local variables to match source type.
]]]
--
Ivan Zhakov
Index: subversion/include/private/svn_adler32.h
===================================================================
--- subversion/include/private/svn_adler32.h (revision 1934976)
+++ subversion/include/private/svn_adler32.h (working copy)
@@ -41,7 +41,7 @@
* @since New in 1.7.
*/
apr_uint32_t
-svn__adler32(apr_uint32_t checksum, const char *data, apr_off_t len);
+svn__adler32(apr_uint32_t checksum, const char *data, apr_size_t len);
#ifdef __cplusplus
Index: subversion/libsvn_diff/diff.h
===================================================================
--- subversion/libsvn_diff/diff.h (revision 1934976)
+++ subversion/libsvn_diff/diff.h (working copy)
@@ -179,7 +179,7 @@
*/
void
svn_diff__normalize_buffer(char **tgt,
- apr_off_t *lengthp,
+ apr_size_t *lengthp,
svn_diff__normalize_state_t *statep,
const char *buf,
const svn_diff_file_options_t *opts);
Index: subversion/libsvn_diff/diff_file.c
===================================================================
--- subversion/libsvn_diff/diff_file.c (revision 1934976)
+++ subversion/libsvn_diff/diff_file.c (working copy)
@@ -748,7 +748,7 @@
char *curp;
char *eol;
apr_off_t last_chunk;
- apr_off_t length;
+ apr_size_t length;
apr_uint32_t h = 0;
/* Did the last chunk end in a CR character? */
svn_boolean_t had_cr = FALSE;
@@ -980,6 +980,8 @@
{
if (length[i] == 0)
{
+ apr_size_t read_length;
+
/* Error if raw_length is 0, that's an unexpected change
* of the file that can happen when ignoring whitespace
* and that can lead to an infinite loop. */
@@ -992,20 +994,21 @@
/* Read a chunk from disk into a buffer */
bufp[i] = buffer[i];
- length[i] = raw_length[i] > COMPARE_CHUNK_SIZE ?
+ read_length = raw_length[i] > COMPARE_CHUNK_SIZE ?
COMPARE_CHUNK_SIZE : raw_length[i];
-
SVN_ERR(read_chunk(file[i]->file,
- bufp[i], length[i], offset[i],
+ bufp[i], read_length, offset[i],
file_baton->pool));
- offset[i] += length[i];
- raw_length[i] -= length[i];
+ length[i] = read_length;
+ offset[i] += read_length;
+ raw_length[i] -= read_length;
/* bufp[i] gets reset to buffer[i] before reading each chunk,
so, overwriting it isn't a problem */
- svn_diff__normalize_buffer(&bufp[i], &length[i], &state[i],
+ svn_diff__normalize_buffer(&bufp[i], &read_length, &state[i],
bufp[i], file_baton->options);
/* assert(length[i] == file_token[i]->length); */
+ length[i] = read_length;
}
}
Index: subversion/libsvn_diff/diff_memory.c
===================================================================
--- subversion/libsvn_diff/diff_memory.c (revision 1934976)
+++ subversion/libsvn_diff/diff_memory.c (working copy)
@@ -132,7 +132,7 @@
char *buf = mem_baton->normalization_buf[0];
svn_string_t *tok = (*token)
= APR_ARRAY_IDX(src->tokens, src->next_token, svn_string_t *);
- apr_off_t len = tok->len;
+ apr_size_t len = tok->len;
svn_diff__normalize_state_t state
= svn_diff__normalize_state_normal;
@@ -158,8 +158,8 @@
svn_string_t *t2 = token2;
char *buf1 = btn->normalization_buf[0];
char *buf2 = btn->normalization_buf[1];
- apr_off_t len1 = t1->len;
- apr_off_t len2 = t2->len;
+ apr_size_t len1 = t1->len;
+ apr_size_t len2 = t2->len;
svn_diff__normalize_state_t state = svn_diff__normalize_state_normal;
svn_diff__normalize_buffer(&buf1, &len1, &state, t1->data,
Index: subversion/libsvn_diff/util.c
===================================================================
--- subversion/libsvn_diff/util.c (revision 1934976)
+++ subversion/libsvn_diff/util.c (working copy)
@@ -145,7 +145,7 @@
void
svn_diff__normalize_buffer(char **tgt,
- apr_off_t *lengthp,
+ apr_size_t *lengthp,
svn_diff__normalize_state_t *statep,
const char *buf,
const svn_diff_file_options_t *opts)
Index: subversion/libsvn_subr/adler32.c
===================================================================
--- subversion/libsvn_subr/adler32.c (revision 1934976)
+++ subversion/libsvn_subr/adler32.c (working copy)
@@ -70,7 +70,7 @@
* of DATA sized LEN.
*/
SVN__ADLER32_STATIC apr_uint32_t
-svn__adler32(apr_uint32_t checksum, const char *data, apr_off_t len)
+svn__adler32(apr_uint32_t checksum, const char *data, apr_size_t len)
{
/* Process large amounts of data in max-sized chunks.
*
