Re: [PATCH] Convert svn_fs_base__get_node_successors() to dual-pool paradigm

[Daniel Shahaf]

Daniel Shahaf wrote on Sun, Sep 11, 2011 at 03:25:18 +0300:
> I wrote this while debugging some pool segfaults, and I don't want to
> commit it until I've resolved them (for obvious reasons), so I'm abusing
> all of your inboxes as my personal patch queueing system.  Thanks for
> your cooperation.
> 
> [[[
> On the fs-successor-ids branch, convert svn_fs_base__get_node_successors()
> to the dual-pool paradigm.
> 
> * subversion/libsvn_fs_base/node-rev.h
>   (svn_fs_base__get_node_successors):
>     Split POOL parameter into RESULT_POOL/SCRATCH_POOL.
> 
> * subversion/libsvn_fs_base/node-rev.c
>   (svn_fs_base__get_node_successors):
>     Convert to the dual-pool paradigm.
> 
> * subversion/libsvn_fs_base/tree.c
>   (txn_body_history_next): Update caller.
> ]]]

[[[
Index: subversion/libsvn_fs_base/tree.c
===================================================================
--- subversion/libsvn_fs_base/tree.c    (revision 1167653)
+++ subversion/libsvn_fs_base/tree.c    (working copy)
@@ -4538,7 +4538,8 @@ txn_body_history_next(void *baton, trail_t *trail)
     SVN_ERR(svn_fs_base__get_node_successors(&successors,
                                              bhd->fs, node_id,
                                              TRUE /* committed_only */,
-                                             trail, pool));
+                                             trail,
+                                             args->pool, trail->pool));
     for (i = 0; i < successors->nelts; i++)
       {
         /* ... then massage its results into something understandable by
Index: subversion/libsvn_fs_base/node-rev.c
===================================================================
--- subversion/libsvn_fs_base/node-rev.c        (revision 1167655)
+++ subversion/libsvn_fs_base/node-rev.c        (working copy)
@@ -164,11 +164,12 @@ svn_fs_base__get_node_successors(apr_array_header_
                                  const svn_fs_id_t *id,
                                  svn_boolean_t committed_only,
                                  trail_t *trail,
-                                 apr_pool_t *pool)
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool)
 {
   apr_array_header_t *all_successors, *successors;
-  apr_pool_t *iterpool = svn_pool_create(pool);
-  svn_string_t *node_id_str = svn_fs_base__id_unparse(id, pool);
+  apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+  svn_string_t *node_id_str = svn_fs_base__id_unparse(id, scratch_pool);
   base_fs_data_t *bfd = fs->fsap_data;
   int i;
 
@@ -180,15 +181,16 @@ svn_fs_base__get_node_successors(apr_array_header_
     }
 
   SVN_ERR(svn_fs_bdb__successors_fetch(&all_successors, fs, node_id_str->data,
-                                       trail, pool));
-  successors = apr_array_make(pool, all_successors->nelts,
+                                       trail, scratch_pool));
+  successors = apr_array_make(result_pool, all_successors->nelts,
                               sizeof(const svn_fs_id_t *));
   for (i = 0; i < all_successors->nelts; i++)
     {
       svn_revnum_t revision;
       const char *succ_id_str = APR_ARRAY_IDX(all_successors, i, const char *);
       const svn_fs_id_t *succ_id = svn_fs_base__id_parse(succ_id_str,
-                                                   strlen(succ_id_str), pool);
+                                                   strlen(succ_id_str),
+                                                   result_pool);
 
       svn_pool_clear(iterpool);
 
Index: subversion/libsvn_fs_base/node-rev.h
===================================================================
--- subversion/libsvn_fs_base/node-rev.h        (revision 1167653)
+++ subversion/libsvn_fs_base/node-rev.h        (working copy)
@@ -106,7 +106,8 @@ svn_error_t *svn_fs_base__get_node_successors(apr_
                                               const svn_fs_id_t *id,
                                               svn_boolean_t committed_only,
                                               trail_t *trail,
-                                              apr_pool_t *pool);
+                                              apr_pool_t *result_pool,
+                                              apr_pool_t *scratch_pool);
 
 #ifdef __cplusplus
 }
]]]