Hyrum Wright wrote:
> On Thu, Sep 9, 2010 at 10:59 AM, <hwright_at_apache.org> wrote:
>
>> Author: hwright
>> Date: Thu Sep 9 15:59:00 2010
>> New Revision: 995478
>>
>> URL: http://svn.apache.org/viewvc?rev=995478&view=rev
>> Log:
>> On the performance branch:
>> Bring up-to-date with trunk.
>>
> ...
>
>> Modified: subversion/branches/performance/subversion/libsvn_subr/io.c
>> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/io.c?rev=995478&r1=995477&r2=995478&view=diff
>> ==============================================================================
>> --- subversion/branches/performance/subversion/libsvn_subr/io.c (original)
>> +++ subversion/branches/performance/subversion/libsvn_subr/io.c Thu Sep 9 15:59:00 2010
>> @@ -1076,14 +1076,19 @@ svn_error_t *svn_io_file_create(const ch
>> {
>> apr_file_t *f;
>> apr_size_t written;
>> + svn_error_t *err;
>>
>> SVN_ERR(svn_io_file_open(&f, file,
>> (APR_WRITE | APR_CREATE | APR_EXCL),
>> APR_OS_DEFAULT,
>> pool));
>> - SVN_ERR(svn_io_file_write_full(f, contents, strlen(contents),
>> - &written, pool));
>> - return svn_io_file_close(f, pool);
>> + err= svn_io_file_write_full(f, contents, strlen(contents),
>> + &written, pool);
>> +
>> +
>> + return svn_error_return(
>> + svn_error_compose_create(err,
>> + svn_io_file_close(f, pool)));
>> }
>>
>> svn_error_t *svn_io_dir_file_copy(const char *src_path,
>> @@ -2952,12 +2957,19 @@ svn_io_write_unique(const char **tmp_pat
>> apr_pool_t *pool)
>> {
>> apr_file_t *new_file;
>> + svn_error_t *err;
>>
>> SVN_ERR(svn_io_open_unique_file3(&new_file, tmp_path, dirpath,
>> delete_when, pool, pool));
>> - SVN_ERR(svn_io_file_write_full(new_file, buf, nbytes, NULL, pool));
>> - SVN_ERR(svn_io_file_flush_to_disk(new_file, pool));
>> - return svn_io_file_close(new_file, pool);
>> +
>> + err = svn_io_file_write_full(new_file, buf, nbytes, NULL, pool);
>> +
>> + if (!err)
>> + err = svn_io_file_flush_to_disk(new_file, pool);
>> +
>> + return svn_error_return(
>> + svn_error_compose_create(err,
>> + svn_io_file_close(new_file, pool)));
>> }
>>
>>
>> @@ -3521,15 +3533,17 @@ svn_io_read_version_file(int *version,
>> apr_file_t *format_file;
>> char buf[80];
>> apr_size_t len;
>> + svn_error_t *err;
>>
>> /* Read a chunk of data from PATH */
>> SVN_ERR(svn_io_file_open(&format_file, path, APR_READ,
>> APR_OS_DEFAULT, pool));
>> len = sizeof(buf);
>> - SVN_ERR(svn_io_file_read(format_file, buf, &len, pool));
>> + err = svn_io_file_read(format_file, buf, &len, pool);
>>
>> /* Close the file. */
>> - SVN_ERR(svn_io_file_close(format_file, pool));
>> + SVN_ERR(svn_error_compose_create(err,
>> + svn_io_file_close(format_file, pool)));
>>
>> /* If there was no data in PATH, return an error. */
>> if (len == 0)
>> @@ -3556,7 +3570,7 @@ svn_io_read_version_file(int *version,
>> }
>>
>> /* Convert to integer. */
>> - *version = atoi(buf);
>> + SVN_ERR(svn_cstring_atoi(version, buf));
>>
>> return SVN_NO_ERROR;
>> }
>> @@ -3570,40 +3584,65 @@ contents_identical_p(svn_boolean_t *iden
>> const char *file2,
>> apr_pool_t *pool)
>> {
>> + svn_error_t *err;
>> apr_size_t bytes_read1, bytes_read2;
>> char *buf1 = apr_palloc(pool, SVN__STREAM_CHUNK_SIZE);
>> char *buf2 = apr_palloc(pool, SVN__STREAM_CHUNK_SIZE);
>> apr_file_t *file1_h = NULL;
>> apr_file_t *file2_h = NULL;
>> + svn_boolean_t done1 = FALSE;
>> + svn_boolean_t done2 = FALSE;
>>
>> SVN_ERR(svn_io_file_open(&file1_h, file1, APR_READ, APR_OS_DEFAULT,
>> pool));
>> - SVN_ERR(svn_io_file_open(&file2_h, file2, APR_READ, APR_OS_DEFAULT,
>> - pool));
>> +
>> + err = svn_io_file_open(&file2_h, file2, APR_READ, APR_OS_DEFAULT,
>> + pool);
>> +
>> + if (err)
>> + return svn_error_return(
>> + svn_error_compose_create(err,
>> + svn_io_file_close(file1_h, pool)));
>>
>> *identical_p = TRUE; /* assume TRUE, until disproved below */
>> - do
>> + while (! (done1 || done2))
>> {
>> - SVN_ERR(svn_io_file_read_full2(file1_h, buf1,
>> - SVN__STREAM_CHUNK_SIZE, &bytes_read1,
>> - TRUE, pool));
>> - SVN_ERR(svn_io_file_read_full2(file2_h, buf2,
>> - SVN__STREAM_CHUNK_SIZE, &bytes_read2,
>> - TRUE, pool));
>> + err = svn_io_file_read_full(file1_h, buf1,
>> + SVN__STREAM_CHUNK_SIZE, &bytes_read1, pool);
>>
>
> Stefan,
> This call, and the one following, needed to revert back to
> svn_io_file_read_full(), since read_full2() was being stuck in an
> infinite loop. Perhaps you can investigate?
>
Hm. That doesn't make any sense. As soon as it tries to read beyond EOF,
the while (bytes_read1==SVN__STREAM_CHUNK_SIZE) condition
should be triggered. Looking through the respective APR code for Windows
and Unix, they always seem to set the number of bytes read even if errors
were encountered.
How did you trigger the infinite loop?
-- Stefan^2.
Received on 2010-09-09 23:35:13 CEST