On Fri, Apr 04, 2003 at 03:22:27PM -0600, kfogel@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_ra_dav/commit.c Fri Apr 4 15:22:22 2003
>...
> @@ -1036,19 +982,14 @@
> put_baton_t *baton;
> svn_stream_t *stream;
>
> - baton = apr_pcalloc(pool, sizeof(*baton));
> - baton->file = file;
> + baton = apr_pcalloc(file->cc->ras->pool, sizeof(*baton));
Woah! That is a session-long pool. This change means that everything will
now get accumulated into one pool, whereas we used to use an iterative pool.
This should be changed to use the file_pool passed to open/add_file.
(which also implies a need to store that pool in the resource_baton_t)
Also, I note that the POOL argument is not documented for apply_textdelta.
Specifically, that it will be destroyed after the "null window" is passed to
the window handler.
>...
> static svn_error_t * commit_close_file(void *file_baton,
> + const char *text_checksum,
> apr_pool_t *pool)
> {
>...
> resource_baton_t *file = file_baton;
> + commit_ctx_t *cc = file->cc;
> + const char *url = file->rsrc->wr_url;
> + ne_request *req;
> + int code;
> + svn_error_t *err;
> +
> + if (file->put_baton)
> + {
> + put_baton_t *pb = file->put_baton;
> +
The variables at the outermost block should move inside, as appropriate.
>...
> + /* run the request and get the resulting status code (and svn_error_t) */
> + err = svn_ra_dav__request_dispatch(&code, req, cc->ras->sess,
> + "PUT", url,
> + 201 /* Created */,
> + 204 /* No Content */,
> + cc->ras->pool);
That should just be "pool".
I'm also guessing that you might be able to fully deref the session, rather
than just to "cc" and further deref'ing several times.
>...
> +++ trunk/subversion/libsvn_ra_dav/fetch.c Fri Apr 4 15:22:22 2003
>...
> @@ -736,8 +735,7 @@
> simple_fetch_file() params related to fetching version URLs (for
> fetching deltas) */
> err = simple_fetch_file(sess, bc_url, NULL, TRUE, compression, file_baton,
> - NULL, (checksum ? checksum->data : NULL),
> - editor, NULL, NULL, pool);
> + NULL, editor, NULL, NULL, pool);
> if (err)
> {
> /* ### do we really need to bother with closing the file_baton? */
> @@ -751,7 +749,7 @@
> err = store_vsn_url(rsrc, file_baton, editor->change_file_prop, pool);
>
> error:
> - err2 = (*editor->close_file)(file_baton, pool);
> + err2 = (*editor->close_file)(file_baton, checksum->data, pool);
> return err ? err : err2;
That error path could choke if checksum==NULL. The previous code had a test
for that.
>...
> +++ trunk/subversion/libsvn_repos/checkout.c Fri Apr 4 15:22:22 2003
> @@ -22,6 +22,8 @@
> #include "svn_path.h"
> #include "svn_pools.h"
>
> +#include <apr_md5.h>
> +#include "svn_md5.h"
Normally, the APR headers are before the SVN headers.
>...
> @@ -453,6 +454,38 @@
>
>
> static svn_error_t *
> +close_file (void *file_baton,
> + const char *text_checksum,
> + apr_pool_t *pool)
> +{
> + struct file_baton *fb = file_baton;
> +
> + if (text_checksum)
> + {
> + unsigned char digest[MD5_DIGESTSIZE];
> + const char *hex_digest;
> +
> + SVN_ERR (svn_fs_file_md5_checksum
> + (digest, fb->edit_baton->txn_root, fb->path, fb->pool));
> + hex_digest = svn_md5_digest_to_cstring (digest, fb->pool);
These operations should use the pool passed to close_file().
Hmm. That pool is doc'd in svn_delta.h either.
>...
> +++ trunk/subversion/libsvn_wc/diff.c Fri Apr 4 15:22:22 2003
>...
> @@ -1053,9 +1052,13 @@
>
> /* An editor function. When the file is closed we have a temporary
> * file containing a pristine version of the repository file. This can
> - * be compared against the working copy. */
> + * be compared against the working copy.
> + *
> + * Ignore TEXT_CHECKSUM.
> + */
Why ignore it? Please explain in the comment, or leave a ### to have this
fixed later on.
(this file is coming from the repository, right? so a checksum error would
be nice to have)
>...
> +++ trunk/subversion/libsvn_wc/status_editor.c Fri Apr 4 15:22:22 2003
>...
> @@ -507,6 +506,7 @@
>
> static svn_error_t *
> close_file (void *file_baton,
> + const char *text_checksum,
> apr_pool_t *pool)
> {
Does the same comment apply here?
>...
> +++ trunk/subversion/libsvn_wc/update_editor.c Fri Apr 4 15:22:22 2003
>...
> @@ -1814,6 +1821,7 @@
> /* Mostly a wrapper around svn_wc_install_file. */
> static svn_error_t *
> close_file (void *file_baton,
> + const char *text_checksum,
> apr_pool_t *pool)
> {
> struct file_baton *fb = file_baton;
> @@ -1824,7 +1832,23 @@
>
> /* window-handler assembles new pristine text in .svn/tmp/text-base/ */
> if (fb->text_changed)
> - new_text_path = svn_wc__text_base_path (fb->path, TRUE, fb->pool);
> + {
> + new_text_path = svn_wc__text_base_path (fb->path, TRUE, fb->pool);
May as well have changed this to just "pool" while you were here.
(it does look like the whole function needs to be switched away from
fb->pool; in fact, the very notion of fb->pool should probably die)
>...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 5 00:59:00 2003