Peter N. Lundblad wrote:
> 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.
I can't change that since I don't have commit access. Brane, can you
please do it?
>>+++ 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.
I'll take this one. Expect a patch soon.
>>+/** 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?
Something like
/** Similar to svn_ra_callbacks2_t, except that the progress
* notification function and baton is missing.
*
* @deprecated Provided for backward compatibility with the 1.2 API.
*/
> Another thought: should we provide a constructor for the new struct so it
> can be extended in the future without having to rev it?
Good idea. I'll take this one too.
>>+++ 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.
Right! Ooops! ;)
I'll fix that one too.
>>+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?)
To make sure that if someone changes that code later for whatever
reason, it's immediately clear what's allowed and what's not.
Aren't comments there for such reasons too?
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 29 18:35:33 2005