You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@galois.collab.net> on 2001/01/29 19:46:30 UTC

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

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);
>    }
>    
>    
>   
>   
>