Re: [PATCH] fix progress callback for ra_svn

Stefan Sperling wrote:
> On Sat, Aug 02, 2008 at 09:40:17AM +0200, Stefan Küng wrote:
>> Hi,
>>
>> Some users reported that TSVN shows unbelievable amounts of bytes
>> transferred for all operations, but only when used with svn://.
>>
>> I've tried ra_serf and ra_neon, those work ok. But ra_svn passes the sum
>> of bytes transferred to the progress callback, while neon and serf pass
>> only the bytes just transferred to the callback (they don't sum up the
>> transferred bytes).
>>
>> I'd like to have all three layers call the progress callback the same
>> way. Since adjusting ra_neon to do the same as ra_svn does would require
>> a change in the neon library, I think it's best to change ra_svn (also,
>> the callback was inspired by the neon progress callback, so I'd say that
>> it's ra_svn which does it wrong).
>>
>> the attached patch fixes this.
>
> The API docs for svn_ra_progress_notify_func_t do not say
> whether the callback should pass the sum of bytes transferred
> so far, or just the amount of bytes transferred since the last
> time the callback was called:
>
> /**
> * Callback function type for progress notification.
> *
> * @a progress is the number of bytes already transferred, @a total is
> * the total number of bytes to transfer or -1 if it's not known, @a
> * baton is the callback baton.
> *
> * @since New in 1.3.
> */
> typedef void (*svn_ra_progress_notify_func_t)(apr_off_t progress,
> apr_off_t total,
> void *baton,
> apr_pool_t *pool);
>
> We should probably make the API docs explicit as well as making
> all RA layers do consistent reporting.

New patch attached, this time with correction to the doc string.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

[[[
count the bytes transferred the same way as ra_serf and ra_neon:
* subversion/include/svn_ra.h: adjust the doc string to make it clear
                               how the callback works
* subversion/libsvn_ra_svn/marshal.c
  (writebuf_output)
  (readbuf_input): do not sum up the bytes transferred
]]]
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 32354)
+++ subversion/include/svn_ra.h (working copy)
@@ -183,9 +183,9 @@
 /**
  * Callback function type for progress notification.
  *
- * @a progress is the number of bytes already transferred, @a total is
- * the total number of bytes to transfer or -1 if it's not known, @a
- * baton is the callback baton.
+ * @a progress is the number of bytes transferred since the last call
+ * of the callback funtion, @a total is the total number of bytes to
+ * transfer or -1 if it's not known, @a baton is the callback baton.
  *
  * @since New in 1.3.
  */
Index: subversion/libsvn_ra_svn/marshal.c
===================================================================
--- subversion/libsvn_ra_svn/marshal.c (revision 32354)
+++ subversion/libsvn_ra_svn/marshal.c (working copy)
@@ -175,7 +175,7 @@
       if (session)
         {
           const svn_ra_callbacks2_t *cb = session->callbacks;
- session->bytes_written += count;
+ session->bytes_written = count;
 
           if (cb && cb->progress_func)
             (cb->progress_func)(session->bytes_written + session->bytes_read,
@@ -266,7 +266,7 @@
   if (session)
     {
       const svn_ra_callbacks2_t *cb = session->callbacks;
- session->bytes_read += *len;
+ session->bytes_read = *len;
 
       if (cb && cb->progress_func)
         (cb->progress_func)(session->bytes_read + session->bytes_written,