One minute after my fix ;) On Sat, Sep 8, 2012 at 12:35 AM, Paul Burba <[email protected]> wrote:
> On Fri, Sep 7, 2012 at 3:29 PM, Julian Foad <[email protected]> > wrote: > > (Just forwarding this thread to Stefan.) > > > > > > - Julian > > > > > > I (Julian Foad) 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 > >>>> @@ -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 */ > > > > p.s. Redundant comment there. > > > >>>> + 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; > > Here is the problem: apr_off_t is unsigned and in some cases we are > trying to calculate a negative offset to pass to svn_io_file_seek. > The attached patch fixes this and all tests pass with it, but I'd > appreciate another set of eyes before committing. > > [[[ > Fix file seek breakage introduced in r1381800. > > See also discussion at http://svn.haxx.se/dev/archive-2012-09/0114.shtml > > * subversion/libsvn_subr/io.c > (svn_io_read_length_line): Move the read/write file offset to an > absolute offset rather than trying to applying a negative offset to > the current offset, thus preventing the mayhem that comes with > assigning a negative offset value to an unsigned apr_off_t. > ]]] > > -- > Paul T. Burba > CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development > Skype: ptburba > > >>>> + *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? > >> > >> Yes, I'm seeing a failure with the same error message, on my Ubuntu > Linux > >> system. > >> > >> - Julian > >> > >> > >>> 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 > >> [...] > -- * 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> ! *
