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

[PATCH] fix string realloc pool mismatches

From: Joe Orton <joe_at_light.plus.com>
Date: 2000-10-09 20:57:02 CEST

Here's the patch which fixes the string-realloc-from-different-pool
thing in libsvn_wc. I don't get any string corruption with this patch,
and without the pool = str->pool fixup from the debugging patch. (And
the debugging patch reports no mismatches)

This is a bad patch though since all the functions called are now
running out of the per-dir pool rather than the per-file pool. But it
does explain where the mismatches are coming from.

The solutions I see to this problem are:

1) start passing >1 pool to these functions
  -> v. confusing, more trouble than it's worth
2) just pass the longer-living pool when necessary (as per patch)
  -> a bit inefficient, a bit confusing to know when to do it
3) use less pools
  -> v. inefficient, but simpler
4) put a pool * in svn_string_t
  -> efficient, clean

I think 4 is the best option (in case you hadn't spotted ;), and though
it is fundamental, should in no way be an unsafe change to make, since
all it does is increase safety?

Regards,

joe

Index: get_editor.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_wc/get_editor.c,v
retrieving revision 1.88
diff -u -U5 -p -r1.88 get_editor.c
--- get_editor.c 2000/10/09 06:20:12 1.88
+++ get_editor.c 2000/10/09 18:34:58
@@ -704,11 +704,11 @@ close_file (void *file_baton)
   apr_status_t apr_err;
   void *local_changes;
   char *version_str = NULL;
   svn_string_t *entry_accum;
 
- err = svn_wc__lock (fb->dir_baton->path, 0, fb->pool);
+ err = svn_wc__lock (fb->dir_baton->path, 0, fb->dir_baton->pool);
   if (err)
     return err;
 
   /* kff todo: if we return before unlocking, which is possible below,
      that is probably badness... */
@@ -803,11 +803,11 @@ close_file (void *file_baton)
       
   err = svn_wc__open_adm_file (&log_fp,
                                fb->dir_baton->path,
                                SVN_WC__ADM_LOG,
                                (APR_WRITE | APR_CREATE), /* not excl */
- fb->pool);
+ fb->dir_baton->pool);
   if (err)
     return err;
 
   /* kff todo: save *local_changes somewhere, maybe to a tmp file
      in SVN/. */
@@ -885,21 +885,21 @@ close_file (void *file_baton)
   /* The log is ready to run, close it. */
   err = svn_wc__close_adm_file (log_fp,
                                 fb->dir_baton->path,
                                 SVN_WC__ADM_LOG,
                                 1, /* sync */
- fb->pool);
+ fb->dir_baton->pool);
   if (err)
     return err;
 
   /* Run the log. */
- err = svn_wc__run_log (fb->dir_baton->path, fb->pool);
+ err = svn_wc__run_log (fb->dir_baton->path, fb->dir_baton->pool);
   if (err)
     return err;
 
   /* Unlock, we're done with this whole file-update. */
- err = svn_wc__unlock (fb->dir_baton->path, fb->pool);
+ err = svn_wc__unlock (fb->dir_baton->path, fb->dir_baton->pool);
   if (err)
     return err;
 
   /* Tell the directory it has one less thing to worry about. */
   err = free_file_baton (fb);
Received on Sat Oct 21 14:36:10 2006

This is an archived mail posted to the Subversion Dev mailing list.