Re: svn commit: r22653 - branches/ra_dav-refactoring/subversion/libsvn_ra_dav

[Daniel Rall <dl...@collab.net>]

On Mon, 11 Dec 2006, dionisos@tigris.org wrote:
...
> --- branches/ra_dav-refactoring/subversion/libsvn_ra_dav/util.c	(original)
> +++ branches/ra_dav-refactoring/subversion/libsvn_ra_dav/util.c	Mon Dec 11 15:43:58 2006
...
> +typedef struct
> +{
> +  svn_ra_dav__request_t *req;
> +  svn_ra_dav__block_reader real_reader;
> +  void *real_baton;
> +} body_reader_wrapper_baton_t;

This structure should indicate that it's a Neon-callback specific
wrapper, in name if not also in doc string.

...
> +static svn_error_t *
> +spool_reader(void *userdata,
> +             const char *buf,
>               size_t len)
>  {
>    spool_reader_baton_t *baton = userdata;
>  
> -  if (! baton->req->err)
> -    baton->req->err = svn_io_file_write_full(baton->spool_file, buf,
> -                                             len, NULL, baton->req->iterpool);
> +  SVN_ERR(svn_io_file_write_full(baton->spool_file, buf,
> +                                 len, NULL, baton->req->iterpool));
>    svn_pool_clear(baton->req->iterpool);
>  
> -  if (baton->req->err)
> -    /* ### Call ne_set_error(), as ne_block_reader doc implies? */
> -    return 1;
> -  else
> -    return 0;
> +  return SVN_NO_ERROR;
>  }

The previous 3 lines could be reduced to:

     return svn_io_file_write_full(...);

...
> +  /* we don't need the status parser: it's attached to the request
> +     and detected errors will be returned there... */
> +  (void)multistatus_parser_create(req);
           ^
Most of Subversion's code uses a space here.

Re: svn commit: r22653 - branches/ra_dav-refactoring/subversion/libsvn_ra_dav

[Daniel Rall <dl...@collab.net>]

On Mon, 11 Dec 2006, Daniel Rall wrote:

> On Mon, 11 Dec 2006, dionisos@tigris.org wrote:
> ...
> > --- branches/ra_dav-refactoring/subversion/libsvn_ra_dav/util.c	(original)
> > +++ branches/ra_dav-refactoring/subversion/libsvn_ra_dav/util.c	Mon Dec 11 15:43:58 2006
...
> > +static svn_error_t *
> > +spool_reader(void *userdata,
> > +             const char *buf,
> >               size_t len)
> >  {
> >    spool_reader_baton_t *baton = userdata;
> >  
> > -  if (! baton->req->err)
> > -    baton->req->err = svn_io_file_write_full(baton->spool_file, buf,
> > -                                             len, NULL, baton->req->iterpool);
> > +  SVN_ERR(svn_io_file_write_full(baton->spool_file, buf,
> > +                                 len, NULL, baton->req->iterpool));
> >    svn_pool_clear(baton->req->iterpool);
> >  
> > -  if (baton->req->err)
> > -    /* ### Call ne_set_error(), as ne_block_reader doc implies? */
> > -    return 1;
> > -  else
> > -    return 0;
> > +  return SVN_NO_ERROR;
> >  }
> 
> The previous 3 lines could be reduced to:
> 
>      return svn_io_file_write_full(...);

Never mind, that's not correct.  My eyes passed right over the clear
of the iteration pool without seeing it.