You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ramkumar Ramachandra <ar...@gmail.com> on 2010/08/19 05:42:14 UTC

[RFC/PATCH] Create a fresh svn_repos_parse_fns3_t

Hi,

I sent a patch a while ago for svn_repos_parse_dumpstream3. While I
wait for approval, this is an RFC patch describing my future plan once
that patch gets approved. It can be described tersely as "While at it
(adding the new callback), fix everything that's wrong with the
struct". I'm planning to fix a few other things as well, but this is
the basic sketch. See load_editor.c for the usecase- I actually
stuffed a scratch pool into the parse_baton so I could use it
everywhere.

-- Ram

Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h	(revision 986884)
+++ subversion/include/svn_repos.h	(working copy)
@@ -2544,6 +2544,129 @@ svn_repos_load_fs(svn_repos_t *repos,
 
 
 /**
+ * A vtable that is driven by svn_repos_parse_dumpstream3().
+ *
+ * @since New in 1.7.
+ */
+typedef struct svn_repos_parse_fns3_t
+{
+  /** The parser has parsed the version information from header of
+   *  the dumpsteeam within the parsing session represented by
+   *  @parse_baton. The version information is available in @a
+   *  version, and a scratch pool @a pool is available for any
+   *  temporary allocations.
+   */
+  svn_error_t *(*dumpstream_version_record)(int version,
+                                            void *parse_baton,
+                                            apr_pool_t *pool);
+
+  /** The parser has discovered a new revision record within the
+   * parsing session represented by @a parse_baton.  All the headers are
+   * placed in @a headers (allocated in @a pool), which maps <tt>const
+   * char *</tt> header-name ==> <tt>const char *</tt> header-value.
+   * The @a revision_baton received back (also allocated in @a pool)
+   * represents the revision.
+   */
+  svn_error_t *(*new_revision_record)(void **revision_baton,
+                                      apr_hash_t *headers,
+                                      void *parse_baton,
+                                      apr_pool_t *pool);
+
+  /** The parser has discovered a new uuid record within the parsing
+   * session represented by @a parse_baton.  The uuid's value is
+   * @a uuid, and it is allocated in @a pool.
+   */
+  svn_error_t *(*uuid_record)(const char *uuid,
+                              void *parse_baton,
+                              apr_pool_t *pool);
+
+  /** The parser has discovered a new node record within the current
+   * revision represented by @a revision_baton.  All the headers are
+   * placed in @a headers (as with @c new_revision_record), allocated in
+   * @a pool.  The @a node_baton received back is allocated in @a pool
+   * and represents the node.
+   */
+  svn_error_t *(*new_node_record)(void **node_baton,
+                                  apr_hash_t *headers,
+                                  void *revision_baton,
+                                  apr_pool_t *pool);
+
+  /** For a given @a revision_baton, set a property @a name to @a
+   *  value. Scratch pool @a pool is available for any temporary
+   *  allocations.
+   */
+  svn_error_t *(*set_revision_property)(void *revision_baton,
+                                        const char *name,
+                                        const svn_string_t *value,
+                                        apr_pool_t *pool);
+
+  /** For a given @a node_baton, set a property @a name to @a
+   *  value. Scratch pool @a pool is available for any temporary
+   *  allocations.
+   */
+  svn_error_t *(*set_node_property)(void *node_baton,
+                                    const char *name,
+                                    const svn_string_t *value,
+                                    apr_pool_t *pool);
+
+  /** For a given @a node_baton, delete property @a name. Scratch pool
+    * @a pool is available for any temporary allocations.
+    */
+  svn_error_t *(*delete_node_property)(void *node_baton,
+                                       const char *name,
+                                       apr_pool_t *pool);
+
+  /** For a given @a node_baton, remove all properties. Scratch pool
+      @a pool is available for any temporary allocations. */
+  svn_error_t *(*remove_node_props)(void *node_baton, apr_pool_t *pool);
+
+  /** For a given @a node_baton, receive a writable @a stream capable of
+   * receiving the node's fulltext.  After writing the fulltext, call
+   * the stream's close() function.
+   *
+   * If a @c NULL is returned instead of a stream, the vtable is
+   * indicating that no text is desired, and the parser will not
+   * attempt to send it.
+   *
+   * Scratch pool @a pool is available for any temporary allocations.
+   */
+  svn_error_t *(*set_fulltext)(svn_stream_t **stream,
+                               void *node_baton,
+                               apr_pool_t *pool);
+
+  /** For a given @a node_baton, set @a handler and @a handler_baton
+   * to a window handler and baton capable of receiving a delta
+   * against the node's previous contents.  A NULL window will be
+   * sent to the handler after all the windows are sent.
+   *
+   * If a @c NULL is returned instead of a handler, the vtable is
+   * indicating that no delta is desired, and the parser will not
+   * attempt to send it.
+   *
+   * Scratch pool @a pool is available for any temporary allocations.
+   */
+  svn_error_t *(*apply_textdelta)(svn_txdelta_window_handler_t *handler,
+                                  void **handler_baton,
+                                  void *node_baton,
+                                  apr_pool_t *pool);
+
+  /** The parser has reached the end of the current node represented by
+   * @a node_baton, it can be freed. Scratch pool @a pool is available
+   * for any temporary allocations.
+   */
+  svn_error_t *(*close_node)(void *node_baton, apr_pool_t *pool);
+
+  /** The parser has reached the end of the current revision
+   * represented by @a revision_baton.  In other words, there are no more
+   * changed nodes within the revision.  The baton can be
+   * freed. Scratch pool @a pool is available for any temporary
+   * allocations.
+   */
+  svn_error_t *(*close_revision)(void *revision_baton, apr_pool_t *pool);
+
+} svn_repos_parse_fns3_t;
+
+/**
  * A vtable that is driven by svn_repos_parse_dumpstream2().
  *
  * @since New in 1.1.

Re: [RFC/PATCH] Create a fresh svn_repos_parse_fns3_t

Posted by Ramkumar Ramachandra <ar...@gmail.com>.
Hi Julian,

Julian Foad writes:
> On Thu, 2010-08-19 at 10:25 +0300, Daniel Shahaf wrote:
> > Ramkumar Ramachandra wrote on Thu, Aug 19, 2010 at 11:12:14 +0530:
> > > Hi,
> > > 
> > > I sent a patch a while ago for svn_repos_parse_dumpstream3. While I
> > > wait for approval, this is an RFC patch describing my future plan once
> > > that patch gets approved. It can be described tersely as "While at it
> > > (adding the new callback), fix everything that's wrong with the
> > > struct".
> 
> I have just read through Daniel's diff of this patch, and the change
> appears to be:
> 
>   Add a dumpstream_version_record() method.
>   Add scratch pool arguments to all methods.
> 
> Is that what you intended?  If so, it sounds sane to me.  (I would name
> the scratch pool arguments 'scratch_pool'.)

Yep, and I'm changing the API to pass the scratch pool around. I'm
using Git to stage all my changes and produce a coherent series with
relevant changes, but it's taking some time- expect to see a series
soon though.

-- Ram

Re: [RFC/PATCH] Create a fresh svn_repos_parse_fns3_t

Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-08-19 at 10:25 +0300, Daniel Shahaf wrote:
> Ramkumar Ramachandra wrote on Thu, Aug 19, 2010 at 11:12:14 +0530:
> > Hi,
> > 
> > I sent a patch a while ago for svn_repos_parse_dumpstream3. While I
> > wait for approval, this is an RFC patch describing my future plan once
> > that patch gets approved. It can be described tersely as "While at it
> > (adding the new callback), fix everything that's wrong with the
> > struct".

I have just read through Daniel's diff of this patch, and the change
appears to be:

  Add a dumpstream_version_record() method.
  Add scratch pool arguments to all methods.

Is that what you intended?  If so, it sounds sane to me.  (I would name
the scratch pool arguments 'scratch_pool'.)

- Julian


Re: [RFC/PATCH] Create a fresh svn_repos_parse_fns3_t

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ramkumar Ramachandra wrote on Thu, Aug 19, 2010 at 11:12:14 +0530:
> Hi,
> 
> I sent a patch a while ago for svn_repos_parse_dumpstream3. While I
> wait for approval, this is an RFC patch describing my future plan once
> that patch gets approved. It can be described tersely as "While at it
> (adding the new callback), fix everything that's wrong with the
> struct". I'm planning to fix a few other things as well, but this is
> the basic sketch. See load_editor.c for the usecase- I actually
> stuffed a scratch pool into the parse_baton so I could use it
> everywhere.
> 
> -- Ram

I derived the diff between the existing/proposed structs.  I think this
form of the RFC will be easier to review:

[[[
--- svn_repos_parse_fns2_t	2010-08-19 10:23:09.000000000 +0300
+++ svn_repos_parse_fns3_t	2010-08-19 10:23:08.000000000 +0300
@@ -1,10 +1,20 @@
 /**
- * A vtable that is driven by svn_repos_parse_dumpstream2().
+ * A vtable that is driven by svn_repos_parse_dumpstream3().
  *
- * @since New in 1.1.
+ * @since New in 1.7.
  */
-typedef struct svn_repos_parse_fns2_t
+typedef struct svn_repos_parse_fns3_t
 {
+  /** The parser has parsed the version information from header of
+   *  the dumpsteeam within the parsing session represented by
+   *  @parse_baton. The version information is available in @a
+   *  version, and a scratch pool @a pool is available for any
+   *  temporary allocations.
+   */
+  svn_error_t *(*dumpstream_version_record)(int version,
+                                            void *parse_baton,
+                                            apr_pool_t *pool);
+
   /** The parser has discovered a new revision record within the
    * parsing session represented by @a parse_baton.  All the headers are
    * placed in @a headers (allocated in @a pool), which maps <tt>const
@@ -36,21 +46,34 @@
                                   void *revision_baton,
                                   apr_pool_t *pool);
 
-  /** For a given @a revision_baton, set a property @a name to @a value. */
+  /** For a given @a revision_baton, set a property @a name to @a
+   *  value. Scratch pool @a pool is available for any temporary
+   *  allocations.
+   */
   svn_error_t *(*set_revision_property)(void *revision_baton,
                                         const char *name,
-                                        const svn_string_t *value);
+                                        const svn_string_t *value,
+                                        apr_pool_t *pool);
 
-  /** For a given @a node_baton, set a property @a name to @a value. */
+  /** For a given @a node_baton, set a property @a name to @a
+   *  value. Scratch pool @a pool is available for any temporary
+   *  allocations.
+   */
   svn_error_t *(*set_node_property)(void *node_baton,
                                     const char *name,
-                                    const svn_string_t *value);
-
-  /** For a given @a node_baton, delete property @a name. */
-  svn_error_t *(*delete_node_property)(void *node_baton, const char *name);
+                                    const svn_string_t *value,
+                                    apr_pool_t *pool);
 
-  /** For a given @a node_baton, remove all properties. */
-  svn_error_t *(*remove_node_props)(void *node_baton);
+  /** For a given @a node_baton, delete property @a name. Scratch pool
+    * @a pool is available for any temporary allocations.
+    */
+  svn_error_t *(*delete_node_property)(void *node_baton,
+                                       const char *name,
+                                       apr_pool_t *pool);
+
+  /** For a given @a node_baton, remove all properties. Scratch pool
+      @a pool is available for any temporary allocations. */
+  svn_error_t *(*remove_node_props)(void *node_baton, apr_pool_t *pool);
 
   /** For a given @a node_baton, receive a writable @a stream capable of
    * receiving the node's fulltext.  After writing the fulltext, call
@@ -59,9 +82,12 @@
    * If a @c NULL is returned instead of a stream, the vtable is
    * indicating that no text is desired, and the parser will not
    * attempt to send it.
+   *
+   * Scratch pool @a pool is available for any temporary allocations.
    */
   svn_error_t *(*set_fulltext)(svn_stream_t **stream,
-                               void *node_baton);
+                               void *node_baton,
+                               apr_pool_t *pool);
 
   /** For a given @a node_baton, set @a handler and @a handler_baton
    * to a window handler and baton capable of receiving a delta
@@ -71,21 +97,26 @@
    * If a @c NULL is returned instead of a handler, the vtable is
    * indicating that no delta is desired, and the parser will not
    * attempt to send it.
+   *
+   * Scratch pool @a pool is available for any temporary allocations.
    */
   svn_error_t *(*apply_textdelta)(svn_txdelta_window_handler_t *handler,
                                   void **handler_baton,
-                                  void *node_baton);
+                                  void *node_baton,
+                                  apr_pool_t *pool);
 
   /** The parser has reached the end of the current node represented by
-   * @a node_baton, it can be freed.
+   * @a node_baton, it can be freed. Scratch pool @a pool is available
+   * for any temporary allocations.
    */
-  svn_error_t *(*close_node)(void *node_baton);
+  svn_error_t *(*close_node)(void *node_baton, apr_pool_t *pool);
 
   /** The parser has reached the end of the current revision
    * represented by @a revision_baton.  In other words, there are no more
-   * changed nodes within the revision.  The baton can be freed.
+   * changed nodes within the revision.  The baton can be
+   * freed. Scratch pool @a pool is available for any temporary
+   * allocations.
    */
-  svn_error_t *(*close_revision)(void *revision_baton);
-
-} svn_repos_parse_fns2_t;
+  svn_error_t *(*close_revision)(void *revision_baton, apr_pool_t *pool);
 
+} svn_repos_parse_fns3_t;
]]]