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

Re: CVS update: subversion/subversion/libsvn_delta xml_output.c

From: Karl Fogel <kfogel_at_galois.collab.net>
Date: 2001-01-29 20:46:30 CET

ghudson@tigris.org writes:
> Log:
> Redo Karl's change to this file. Instead of ref-counting directory
> batons, keep a count of open files in edit_context. Open issue: as
> with Karl's approach, svn_delta_get_xml_editor generates some output
> before the first editor call; I'm not sure I consider that within the
> contract.

I think it is. We already were handing the output stream to the
editor at creation time, not at replace_root() time. I don't think
there was ever a promise that the editor would ignore the stream until
replace_root().

The documentation for replace_root() merely promised that it would
return a directory baton for the root directory. It didn't actually
make promises about doing things, nor did the editor make promises
about not doing things before replace_root() was called.

-K

> Relative to rev 1.24:
> (replace_root): Removed.
> (close_edit): No longer an editor function; accepts a struct
> edit_context * instead of a void *.
> (edit_context): Replaces edit_baton. All references changed.
> Add open_file_count and root_dir_closed fields to track when the edit
> is over.
> (close_directory): Note when the root dir is closed, and close the
> edit if there are no open files at that point.
> (add_file, replace_file): Bump open_file_count.
> (close_file): Decrement open_file_count, and close the edit if the
> root dir has been closed and there are no open files.
> (tree_editor): Remove entries for replace_root and close_edit.
> (svn_delta_get_xml_editor, close_edit): Don't use a subpool for the
> editor. We still use subpools for the directory and file batons
> because they use a large, unbounded amount of storage together, but
> there's no need for a subpool for the editor itself.
> (svn_delta_get_xml_editor): Conform to new interface.
>
> Revision Changes Path
> 1.26 +60 -170 subversion/subversion/libsvn_delta/xml_output.c
>
> Index: xml_output.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_delta/xml_output.c,v
> retrieving revision 1.25
> retrieving revision 1.26
> diff -u -r1.25 -r1.26
> --- xml_output.c 2001/01/27 02:36:32 1.25
> +++ xml_output.c 2001/01/27 05:43:30 1.26
> @@ -65,6 +65,8 @@
> struct file_baton *curfile;
> apr_pool_t *pool;
> int txdelta_id_counter;
> + int open_file_count;
> + svn_boolean_t root_dir_closed;
> };
>
>
> @@ -74,17 +76,6 @@
> enum elemtype addreplace; /* elem_add or elem_replace, or
> elem_delta_pkg for the root
> directory. */
> -
> - /* Baton for this dir's parent directory, null if this is root. */
> - struct dir_baton *parent_dir_baton;
> -
> - /* The number of other changes associated with this directory.
> - Starts at 1, is incremented for each file and subdir opened here,
> - and decremented for each one closed. When ref_count reaches 0,
> - final cleanups may be performed and the directory freed. See
> - free_dir_baton() for details. */
> - int ref_count;
> -
> apr_pool_t *pool;
> };
>
> @@ -97,17 +88,12 @@
> 0 means we're still working on the file,
> -1 means we already saw a text delta. */
> int closed; /* 1 if we closed the element already. */
> -
> - struct dir_baton *parent_dir_baton; /* This file's parent directory. */
> -
> apr_pool_t *pool;
> };
>
>
> static struct dir_baton *
> -make_dir_baton (struct edit_context *ec,
> - struct dir_baton *parent_dir_baton,
> - enum elemtype addreplace)
> +make_dir_baton (struct edit_context *ec, enum elemtype addreplace)
> {
> apr_pool_t *subpool = svn_pool_create (ec->pool);
> struct dir_baton *db = apr_palloc (subpool, sizeof (*db));
> @@ -115,20 +101,12 @@
> db->edit_context = ec;
> db->addreplace = addreplace;
> db->pool = subpool;
> - db->ref_count = 1;
> - db->parent_dir_baton = parent_dir_baton;
> -
> - if (parent_dir_baton)
> - parent_dir_baton->ref_count++;
> -
> return db;
> }
>
>
> static struct file_baton *
> -make_file_baton (struct edit_context *ec,
> - struct dir_baton *parent_dir_baton,
> - enum elemtype addreplace)
> +make_file_baton (struct edit_context *ec, enum elemtype addreplace)
> {
> apr_pool_t *subpool = svn_pool_create (ec->pool);
> struct file_baton *fb = apr_palloc (subpool, sizeof (*fb));
> @@ -138,81 +116,10 @@
> fb->txdelta_id = 0;
> fb->closed = 0;
> fb->pool = subpool;
> - fb->parent_dir_baton = parent_dir_baton;
> -
> - if (parent_dir_baton)
> - parent_dir_baton->ref_count++;
> -
> return fb;
> }
>
>
> -/* Prototypes. */
> -static svn_error_t *decrement_ref_count (struct dir_baton *d);
> -static svn_string_t *get_to_elem (struct edit_context *ec,
> - enum elemtype elem,
> - apr_pool_t *pool);
> -
> -
> -static svn_error_t *
> -free_dir_baton (struct dir_baton *db)
> -{
> - struct dir_baton *pb = db->parent_dir_baton;
> - svn_error_t *err;
> -
> - if (pb)
> - {
> - apr_destroy_pool (db->pool);
> -
> - err = decrement_ref_count (pb);
> - if (err)
> - return err;
> - }
> - else
> - {
> - /* We're freeing the root directory. Most of the work was
> - done long ago by close_directory(), but there's some
> - end-of-edit bookkeeping to take care of now. */
> -
> - svn_string_t *str = NULL;
> - apr_size_t len;
> -
> - svn_xml_make_close_tag (&str, db->edit_context->pool, "delta-pkg");
> - len = str->len;
> - err = svn_stream_write (db->edit_context->output, str->data, &len);
> - if (err == SVN_NO_ERROR)
> - err = svn_stream_close (db->edit_context->output);
> -
> - apr_destroy_pool (db->edit_context->pool); /* destroys db->pool too */
> - }
> -
> - if (err)
> - return err;
> -
> - return SVN_NO_ERROR;
> -}
> -
> -
> -/* Decrement DIR_BATON's ref count, and if the count hits 0, call
> - * free_dir_baton().
> - *
> - * Note: There is no corresponding function for incrementing the
> - * ref_count. As far as we know, nothing special depends on that, so
> - * it's always done inline.
> - */
> -static svn_error_t *
> -decrement_ref_count (struct dir_baton *db)
> -{
> - db->ref_count--;
> -
> - if (db->ref_count == 0)
> - return free_dir_baton (db);
> -
> - return SVN_NO_ERROR;
> -}
> -
> -
> -
> /* The meshing between the edit_fns interface and the XML delta format
> is such that we can't usually output the end of an element until we
> go on to the next thing, and for a given call we may or may not
> @@ -415,7 +322,7 @@
> struct dir_baton *db = (struct dir_baton *) parent_baton;
> struct edit_context *ec = db->edit_context;
>
> - *child_baton = make_dir_baton (ec, db, elem_add);
> + *child_baton = make_dir_baton (ec, elem_add);
> return output_addreplace (ec, elem_add, elem_dir, name,
> ancestor_path, ancestor_revision);
> }
> @@ -431,7 +338,7 @@
> struct dir_baton *db = (struct dir_baton *) parent_baton;
> struct edit_context *ec = db->edit_context;
>
> - *child_baton = make_dir_baton (ec, db, elem_replace);
> + *child_baton = make_dir_baton (ec, elem_replace);
> return output_addreplace (ec, elem_replace, elem_dir, name,
> ancestor_path, ancestor_revision);
> }
> @@ -454,44 +361,31 @@
> {
> struct dir_baton *db = (struct dir_baton *) dir_baton;
> struct edit_context *ec = db->edit_context;
> - svn_string_t *str = NULL;
> - svn_error_t *err;
> + svn_string_t *str;
> apr_size_t len;
>
> + str = get_to_elem (ec, elem_dir, db->pool);
> if (db->addreplace != elem_delta_pkg)
> {
> - /* Not the root directory. */
> + /* Not the root directory. */
> const char *outertag = (db->addreplace == elem_add) ? "add" : "replace";
> - str = get_to_elem (ec, elem_dir, db->pool);
> svn_xml_make_close_tag (&str, db->pool, "dir");
> svn_xml_make_close_tag (&str, db->pool, outertag);
> ec->elem = elem_tree_delta;
> -
> - len = str->len;
> - err = svn_stream_write (ec->output, str->data, &len);
> - if (err)
> - return err;
> - }
> - else /* root directory */
> - {
> - /* Unconditionally output the "</tree-delta>" for the root
> - * directory, since the only things that could possibly follow
> - * this call should also follow that close tag.
> - *
> - * kff todo: my solution here is an offensive kluge. I hope
> - * that Greg Hudson, with his better understanding of the XML
> - * output editor, will show me The Way. */
> - svn_xml_make_close_tag (&str, db->pool, "tree-delta");
> - len = str->len;
> - err = svn_stream_write (ec->output, str->data, &len);
> - if (err)
> - return err;
> }
> -
> - err = decrement_ref_count (db);
> - if (err)
> - return err;
> + else
> + {
> + /* We're closing the root directory. */
> + ec->elem = elem_delta_pkg;
> + ec->root_dir_closed = TRUE;
> + }
>
> + len = str->len;
> + if (len != 0)
> + SVN_ERR (svn_stream_write (ec->output, str->data, &len));
> + apr_destroy_pool (db->pool);
> + if (ec->root_dir_closed && ec->open_file_count == 0)
> + SVN_ERR (close_edit (ec));
> return SVN_NO_ERROR;
> }
>
> @@ -508,8 +402,9 @@
>
> SVN_ERR(output_addreplace (ec, elem_add, elem_file, name,
> ancestor_path, ancestor_revision));
> - *file_baton = make_file_baton (ec, db, elem_add);
> + *file_baton = make_file_baton (ec, elem_add);
> ec->curfile = *file_baton;
> + ec->open_file_count++;
> return SVN_NO_ERROR;
> }
>
> @@ -526,8 +421,9 @@
>
> SVN_ERR(output_addreplace (ec, elem_replace, elem_file, name,
> ancestor_path, ancestor_revision));
> - *file_baton = make_file_baton (ec, db, elem_replace);
> + *file_baton = make_file_baton (ec, elem_replace);
> ec->curfile = *file_baton;
> + ec->open_file_count++;
> return SVN_NO_ERROR;
> }
>
> @@ -635,10 +531,8 @@
> close_file (void *file_baton)
> {
> struct file_baton *fb = (struct file_baton *) file_baton;
> - struct dir_baton *pb = fb->parent_dir_baton;
> struct edit_context *ec = fb->edit_context;
> svn_string_t *str;
> - svn_error_t *err = SVN_NO_ERROR;
> apr_size_t len;
>
> /* Close the file element if we are still working on it. */
> @@ -650,22 +544,30 @@
> svn_xml_make_close_tag (&str, fb->pool, outertag);
>
> len = str->len;
> - err = svn_stream_write (ec->output, str->data, &len);
> + SVN_ERR (svn_stream_write (ec->output, str->data, &len));
> ec->curfile = NULL;
> ec->elem = elem_tree_delta;
> }
> + apr_destroy_pool (fb->pool);
>
> - if (err)
> - return err;
> + ec->open_file_count--;
> + if (ec->root_dir_closed && ec->open_file_count == 0)
> + SVN_ERR (close_edit (ec));
> + return SVN_NO_ERROR;
> +}
>
> - /* Free the file baton. */
> - apr_destroy_pool (fb->pool);
>
> - /* Tell parent its gone. */
> - err = decrement_ref_count (pb);
> - if (err)
> - return err;
> +static svn_error_t *
> +close_edit (struct edit_context *ec)
> +{
> + svn_error_t *err;
> + svn_string_t *str = NULL;
> + apr_size_t len;
>
> + svn_xml_make_close_tag (&str, ec->pool, "delta-pkg");
> + len = str->len;
> + SVN_ERR (svn_stream_write (ec->output, str->data, &len));
> + SVN_ERR (svn_stream_close (ec->output));
> return SVN_NO_ERROR;
> }
>
> @@ -691,43 +593,31 @@
> void **root_dir_baton,
> apr_pool_t *pool)
> {
> - struct dir_baton *rb;
> struct edit_context *ec;
> - apr_pool_t *subpool = svn_pool_create (pool);
> -
> - *editor = &tree_editor;
> + svn_string_t *str = NULL;
> + apr_size_t len;
>
> - /* Set up an edit_context. */
> - ec = apr_palloc (subpool, sizeof (*ec));
> - ec->pool = subpool;
> + /* Construct and initialize the editor context. */
> + ec = apr_palloc (pool, sizeof (*ec));
> + ec->pool = pool;
> ec->output = output;
> + ec->elem = elem_dir;
> ec->curfile = NULL;
> ec->txdelta_id_counter = 1;
> + ec->open_file_count = 0;
> + ec->root_dir_closed = FALSE;
>
> - /* Set up a root_dir_baton, initialized with that edit_context, and
> - output the start of the xml. */
> - {
> - svn_error_t *err;
> - svn_string_t *str = NULL;
> - apr_size_t len;
> - apr_pool_t *this_pool = svn_pool_create (subpool);
> -
> - svn_xml_make_header (&str, this_pool);
> - svn_xml_make_open_tag (&str, this_pool, svn_xml_normal, "delta-pkg", NULL);
> -
> - rb = make_dir_baton (ec, NULL, elem_delta_pkg);
> - ec->elem = elem_dir;
> -
> - len = str->len;
> - err = svn_stream_write (ec->output, str->data, &len);
> - apr_destroy_pool (this_pool);
> -
> - if (err)
> - return err;
> - }
> + /* Now set up the editor and root baton for the caller. */
> + *editor = &tree_editor;
> + *root_dir_baton = make_dir_baton (ec, elem_delta_pkg);
>
> - *root_dir_baton = rb;
> - return SVN_NO_ERROR;
> + /* Construct and write out the header. This should probably be
> + deferred until the first editor call. */
> + svn_xml_make_header (&str, pool);
> + svn_xml_make_open_tag (&str, pool, svn_xml_normal, "delta-pkg", NULL);
> +
> + len = str->len;
> + return svn_stream_write (ec->output, str->data, &len);
> }
>
>
>
>
>
Received on Sat Oct 21 14:36:20 2006

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.