Re: svn commit: r39598 - in trunk/subversion: include libsvn_wc

Internal pools are evil, and lead to significant troubles when not handled
properly.

I'd suggest treating conflicts as immutable, thus never needing further
allocs or an internal pool.

Cheers,
-g

On Oct 19, 2009 12:40 PM, "Julian Foad" <julianfoad_at_btopenworld.com> wrote:

On Fri, 2009-09-25, Bert Huijben wrote: > Author: rhuijben > Date: Fri Sep
25 06:14:56 2009 > New Re...
Problem below...

> Copied and modified: trunk/subversion/libsvn_wc/conflicts.c (from r39596,
trunk/subversion/libsvn...

> + * conflicts.c: routines for managing conflict data. > + * NOTE: this
code doesn't kno...

> +struct svn_wc_conflict_t > +{ > + /* Pool conflict is allocated in */ > +
apr_pool_t *pool; > ...
> + /* ### TODO: Add more fields */
> +};

> +svn_error_t * > +svn_wc_conflict_dup(svn_wc_conflict_t** duplicate, > +
svn_...
> {
> + svn_wc_conflict_t *c;
> + if (result_pool == base->pool)
> {
> + /* No need to duplicate; base has the same liftime and its inner
> + values can't change */

... This is trying to be too clever. The doc string promises to return a
copy of the structure, allocated in result_pool. If it's important to
implement such an optimisation (and I don't know that it is) the
doc-string should allow it.

> + *duplicate = base;
> + return SVN_NO_ERROR;
> }
> +
> + SVN_ERR(conflict_alloc(&c, result_pool));
>
> + c->kind = base->kind;
> + c->property_name = base->property_name
> + ? apr_pstrdup(result_pool, base->property_name)
> + : NULL;
>
> + *duplicate = c;
> return SVN_NO_ERROR;
> }

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2409028

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409059
Received on 2009-10-19 19:46:25 CEST