One minute after my fix ;)
On Sat, Sep 8, 2012 at 12:35 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Fri, Sep 7, 2012 at 3:29 PM, Julian Foad <julianfoad_at_btopenworld.com>
> wrote:
> > (Just forwarding this thread to Stefan.)
> >
> >
> > - Julian
> >
> >
> > I (Julian Foad) wrote:
> >
> >> Paul Burba wrote:
> >>
> >>> On Thu, Sep 6, 2012 at 7:32 PM, <stefan2_at_apache.org> 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>
!
*
Received on 2012-09-08 00:48:31 CEST