[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Completed TODO: svn_stream_copy3()

From: Greg Stein <gstein_at_gmail.com>
Date: Sun, 12 Apr 2009 20:10:00 +0200

Committed in r37188 with some minor tweaks (removed initializers on
err/err2, a blank line, and the extra parens around (err)).

Thanks for the patch!

Cheers,
-g

On Fri, Apr 10, 2009 at 18:06, Edmund Wong <ed_at_kdtc.net> wrote:
> Hi again,
>
> Here's another attempt at this patch.  I took the
> liberty of doing the following:
>
>   if ((err) || (len != SVN__STREAM_CHUNK_SIZE))
>       break;
>
> Is this ok?  Or is this a no-no.  So far I haven't
> seen any of the real code (aside for merge.c),
> so I haven't seen evidence that leads to a different
> conclusion that this construct is invalid or
> illegal(in terms of SVN coding conduct).
>
> Thanks
>
> Edmund
>
> Log:
>
> [[[
>          Completed the TODO as stated in svn_io.h for
>       svn_stream_copy3().
>
>       * subversion/libsvn_subr/stream.c
>         (svn_stream_copy3): Even on error exit, the two files
>                             from and to are closed.
>
>        Patch by: Edmund Wong <edmund_at_belfordhk.com>
>
>        Review by: mf
>                   gstein
> ]]]
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1632483
> Index: subversion/libsvn_subr/stream.c
> ===================================================================
> --- subversion/libsvn_subr/stream.c     (revision 37129)
> +++ subversion/libsvn_subr/stream.c     (working copy)
> @@ -210,6 +210,8 @@
>                               apr_pool_t *scratch_pool)
>  {
>   char *buf = apr_palloc(scratch_pool, SVN__STREAM_CHUNK_SIZE);
> +  svn_error_t *err = SVN_NO_ERROR;
> +  svn_error_t *err2 = SVN_NO_ERROR;
>
>   /* Read and write chunks until we get a short read, indicating the
>      end of the stream.  (We can't get a short write without an
> @@ -219,17 +221,28 @@
>       apr_size_t len = SVN__STREAM_CHUNK_SIZE;
>
>       if (cancel_func)
> -        SVN_ERR(cancel_func(cancel_baton));
> +        {
> +          err = cancel_func(cancel_baton);
>
> -      SVN_ERR(svn_stream_read(from, buf, &len));
> +          if (err)
> +             break;
> +        }
> +
> +      err = svn_stream_read(from, buf, &len);
> +      if (err)
> +        break;
> +
>       if (len > 0)
> -        SVN_ERR(svn_stream_write(to, buf, &len));
> -      if (len != SVN__STREAM_CHUNK_SIZE)
> -        break;
> +        err = svn_stream_write(to, buf, &len);
> +
> +      if ((err) || (len != SVN__STREAM_CHUNK_SIZE))
> +          break;
>     }
> +
> +  err2 = svn_error_compose_create(svn_stream_close(from),
> +                                  svn_stream_close(to));
>
> -  return svn_error_compose_create(svn_stream_close(from),
> -                                  svn_stream_close(to));
> +  return svn_error_compose_create(err, err2);
>  }
>
>  svn_error_t *
>
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1673083
Received on 2009-04-12 20:10:16 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.