> -----Original Message-----
> From: Daniel Shahaf [mailto:d...@daniel.shahaf.name]
> Sent: dinsdag 15 maart 2011 5:47
> To: dev@subversion.apache.org
> Cc: comm...@subversion.apache.org
> Subject: Re: svn commit: r1079686 -
> /subversion/trunk/subversion/libsvn_diff/parse-diff.c
> 
> style...@apache.org wrote on Wed, Mar 09, 2011 at 07:40:38 -0000:
> > Author: stylesen
> > Date: Wed Mar  9 07:40:38 2011
> > New Revision: 1079686
> >
> > URL: http://svn.apache.org/viewvc?rev=1079686&view=rev
> > Log:
> > Clean up some deprecated functions.
> >
> > * subversion/libsvn_diff/parse-diff.c
> >   (scan_eol, readline): Use svn_io_file_read_full2 which makes finding
EOF
> >    simpler.
> >
> > Modified:
> >     subversion/trunk/subversion/libsvn_diff/parse-diff.c
> >
> > Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/pars
> e-diff.c?rev=1079686&r1=1079685&r2=1079686&view=diff
> >
> ==========================================================
> ====================
> > --- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
> > +++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Wed Mar  9
> 07:40:38 2011
> > @@ -287,18 +287,15 @@ scan_eol(const char **eol, apr_file_t *f
> >      {
> >        char buf[256];
> >        apr_size_t len;
> > -      svn_error_t *err;
> > +      svn_boolean_t eof = FALSE;
> >
> 
> You don't need to initialize EOF.
> 
> Also, you don't actually *read* that variable anywhere, so you should
> drop it entirely :-)
> 
> >        if (total_len >= max_len)
> >          break;
> >
> >        len = sizeof(buf) - 1 < (max_len - total_len) ? sizeof(buf) - 1
> >                                                      : (max_len -
total_len);
> > -      err = svn_io_file_read_full(file, buf, sizeof(buf) - 1, &len,
pool);
> > -      if (err && APR_STATUS_IS_EOF(err->apr_err))
> > -        svn_error_clear(err);
> > -      else
> > -        SVN_ERR(err);
> > +      SVN_ERR(svn_io_file_read_full2(file, buf, sizeof(buf) - 1, &len,
&eof,
> > +                                     pool));
> 
> sizeof(buf)-1 or sizeof(buf)?  The next call passes sizeof(c) (which,
> there, is a char).

This really needs sizeof(buf)-1, to allow adding a final 0 byte after the
read bytes a few lines lower.  So we write a 0 outside our allocated memory.

This part of the change breaks the Windows buildbot. Committing a fix in a
few minutes.

        Bert

Reply via email to