On Fri, Sep 7, 2012 at 9:18 PM, Julian Foad <[email protected]>wrote:
> Paul Burba wrote: > > > On Thu, Sep 6, 2012 at 7:32 PM, <[email protected]> wrote: > >> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev > >> Log: > >> Re-implement svn_io_read_length_line as this is one of the > >> most-called functions in SVN. Instead of reading data a byte > >> at a time, read 128 byte chunks and scan those. > [...] > > > ============================================================================== > >> --- subversion/trunk/subversion/libsvn_subr/io.c (original) > >> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11 > 2012 > >> @@ -3427,30 +3427,60 @@ svn_error_t * > >> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t > *limit, > >> apr_pool_t *pool) > >> { > >> + /* variables */ > >> + apr_size_t total_read = 0; > >> + svn_boolean_t eof = FALSE; > >> const char *name; > >> svn_error_t *err; > >> - apr_size_t i; > >> - char c; > >> + apr_size_t buf_size = *limit; > >> > >> - for (i = 0; i < *limit; i++) > >> + while (buf_size > 0) > >> { > >> - SVN_ERR(svn_io_file_getc(&c, file, pool)); > >> - /* Note: this error could be APR_EOF, which > >> - is totally fine. The caller should be aware of > >> - this. */ > >> - > >> - if (c == '\n') > >> + /* read a fair chunk of data at once. But don't get too > ambitious > >> + * as that would result in too much waste. Also make sure we can > >> + * put a NUL after the last byte read. > >> + */ > >> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128; > >> + apr_size_t bytes_read = 0; > >> + char *eol; > >> + > >> + /* read data block (or just a part of it) */ > >> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read, > >> + &bytes_read, &eof, pool)); > >> + > >> + /* look or a newline char */ > >> + buf[bytes_read] = 0; > >> + eol = strchr(buf, '\n'); > >> + if (eol) > >> { > >> - buf[i] = '\0'; > >> - *limit = i; > >> + apr_off_t offset = (eol + 1 - buf) - bytes_read; > >> + > >> + *eol = 0; > >> + *limit = total_read + (eol - buf); > >> + > >> + /* correct the file pointer: > >> + * appear as though we just had read the newline char > >> + */ > >> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool)); > >> + > >> return SVN_NO_ERROR; > >> } > >> - else > >> + else if (eof) > >> { > >> - buf[i] = c; > >> + /* no EOL found but we hit the end of the file. > >> + * Generate a nice EOF error object and return it. > >> + */ > >> + char dummy; > >> + SVN_ERR(svn_io_file_getc(&dummy, file, pool)); > >> } > >> + > >> + /* next data chunk */ > >> + buf_size -= bytes_read; > >> + buf += bytes_read; > >> + total_read += bytes_read; > >> } > >> > >> + /* buffer limit has been exceeded without finding the EOL */ > >> err = svn_io_file_name_get(&name, file, pool); > >> if (err) > >> name = NULL; > >> > >> > > > > Anybody else seeing this? > Thanks for the report, Paul! Having no buildbots when one needs them is a nuisance. > Yes, I'm seeing a failure with the same error message, on my Ubuntu Linux > system. > Thanks for testing. I had no issues on 64 bit Ubuntu. => the issue must have been a 32 bit specific one. > > > I haven't figured out why yet, but r1381800 is causing failures on my > > Windows box, all similar to this: > [...] > > I: svn: E070014: Can't read file > > > 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0': > > End of file found > [...] > Fixed in r1382196. -- Stefan^2. -- * Join us this October at Subversion Live 2012<http://www.wandisco.com/svn-live-2012> for two days of best practice SVN training, networking, live demos, committer meet and greet, and more! Space is limited, so get signed up today<http://www.wandisco.com/svn-live-2012> ! *
