Re: svn commit: r995478 [1/2] - in /subversion/branches/performance: ./ subversion/bindings/javahl/native/ subversion/bindings/javahl/src/org/apache/subversion/javahl/ subversion/bindings/javahl/src/org/tigris/subversion/javahl/ subversion/bindings/j

[Stefan Fuhrmann]

Hyrum Wright wrote:

On Thu, Sep 9, 2010 at 10:59 AM,  <hwri...@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.