On Fri, 10 Jun 2005, [UTF-8] Tobias Ringström wrote:
> lundblad@tigris.org wrote:
>
> >Modified: trunk/subversion/libsvn_fs_fs/fs_fs.c
> >Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=15016&p1=trunk/subversion/libsvn_fs_fs/fs_fs.c&p2=trunk/subversion/libsvn_fs_fs/fs_fs.c&r1=15015&r2=15016
> >==============================================================================
> >--- trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> >+++ trunk/subversion/libsvn_fs_fs/fs_fs.c Thu Jun 9 16:29:10 2005
> >@@ -2023,7 +2023,7 @@
> > old_change->text_mod = change->text_mod;
> > old_change->prop_mod = change->prop_mod;
> > if (change->copyfrom_rev == SVN_INVALID_REVNUM)
> >- copyfrom_string = NULL;
> >+ copyfrom_string = apr_pstrdup (copyfrom_pool, "");
> > else
> > {
> > copyfrom_string = apr_psprintf (copyfrom_pool,
> >@@ -2062,7 +2062,7 @@
> > change->copyfrom_path);
> > }
> > else
> >- copyfrom_string = NULL;
> >+ copyfrom_string = apr_pstrdup (copyfrom_pool, "");
> > path = apr_pstrdup (pool, change->path);
> > }
> >
> >
> >
> >
> Thanks for fixing my error, Peter, but may I suggest that you don't put
> the old (weird) strdup of the empty string back in. Just do
>
> copyfrom_string = "";
>
Well, I guess it doesn't matter that much in this case; we wate about 8
bytes (or something) per allocation.
Normally, when we return something "allocated in @a pool", we should
really allocate it from said pool (this is important for public PIs).
Since we, as a dynamically loadable library, can be unloaded before the
pool is destroyed, even static data may have a too short lifetime. In the
case at hand, we have an internal cache, but why introduce the possibility
for subtle bugs in the future?
(We had a similar discussion some time ago when I introduced
svn_cmdline_output_encoding()). Maybe we should add something to HACKING
about returning pointers to static data? I'll see if there is an
appropriate place
Regards and thanks for the review,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jun 13 15:10:08 2005