On Sun, 28 Aug 2005 brane@tigris.org wrote:
> Log:
> Implement a reduced version of issue #901 for DAV connections.
>
So, one has to look up the issue to know what this patch is doing. One
more sentence telling what the change is for would be good.
>
> +++ trunk/subversion/include/svn_ra.h Sun Aug 28 08:03:41 2005
> @@ -169,6 +169,19 @@
> svn_error_t *ra_err,
> apr_pool_t *pool);
>
> +/**
> + * 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);
> +
Space after star, and this function should have a pool argument for
temporary allocations.
> +/** A collection of callbacks implemented by libsvn_client which allows
> + * an RA layer to "pull" information from the client application, or
> + * possibly store information. libsvn_client passes this vtable to
> + * @c RA->open().
> + *
> + * Each routine takes a @a callback_baton originally provided with the
> + * vtable.
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> */
> typedef struct svn_ra_callbacks_t
Should change the above docs to just say "Similar to ... except ...". And
could we get rid of the duplicate documentation for these fields?
Another thought: should we provide a constructor for the new struct so it
can be extended in the future without having to rev it?
> +++ trunk/subversion/libsvn_ra/ra_loader.c Sun Aug 28 08:03:41 2005
> +svn_error_t *svn_ra_open (svn_ra_session_t **session_p,
> + const char *repos_URL,
> + const svn_ra_callbacks_t *callbacks,
> + void *callback_baton,
> + apr_hash_t *config,
> + apr_pool_t *pool)
> +{
> + /* Ddeprecated function. Copy the contents of the svn_ra_callbacks_t
> + to a new svn_ra_callbacks2_t and call svn_ra_open2(). */
> + svn_ra_callbacks2_t callbacks2;
Ooops! Stack smasher. callbacks2 allocated on the stack; used beyond by
the session.
>
> Modified: trunk/subversion/libsvn_ra/wrapper_template.h
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_ra/wrapper_template.h?rev=15948&p1=trunk/subversion/libsvn_ra/wrapper_template.h&p2=trunk/subversion/libsvn_ra/wrapper_template.h&r1=15947&r2=15948
> ==============================================================================
> --- trunk/subversion/libsvn_ra/wrapper_template.h (original)
> +++ trunk/subversion/libsvn_ra/wrapper_template.h Sun Aug 28 08:03:41 2005
> @@ -45,9 +45,20 @@
> apr_pool_t *pool)
> {
> svn_ra_session_t *sess = apr_pcalloc (pool, sizeof (svn_ra_session_t));
> + svn_ra_callbacks2_t callbacks2;
Similar.
> +static void
> +ra_dav_neonprogress(void * baton, off_t progress, off_t total)
> +{
> + /* Important: don't change the svn_ra_callbacks2_t struct here! */
So, that's why it is const below? (Point is: why this comment?)
> + const svn_ra_callbacks2_t * callbacks = (svn_ra_callbacks2_t *)baton;
> + if (callbacks->progress_func)
> + {
> + callbacks->progress_func(progress, total, callbacks->progress_baton);
> + }
> +}
>
>
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 29 14:31:25 2005