You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@finemaltcoding.com> on 2005/10/04 19:07:09 UTC
libsvn_wc/status.c:assemble_status() doc string lies?
assemble_status() produces a svn_wc_status2_t. Part of its input for
assembly of that structure is a svn_wc_entry_t. About that entry, it
claims:
ENTRY may be null, for non-versioned entities. In this case, we
will assemble a special status structure item which implies a
non-versioned thing.
Else, ENTRY's pool must not be shorter-lived than STATUS's, since
ENTRY will be stored directly, not copied. <==== *
However, it appears to do a deep copy of entry:
stat->entry = svn_wc_entry_dup (entry, pool);
Re: libsvn_wc/status.c:assemble_status() doc string lies?
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Tue, 04 Oct 2005, Philip Martin wrote:
> kfogel@collab.net writes:
...
> > A bit verbose, maybe just:
> >
> > + non-versioned thing. If non-null, ENTRY's lifetime is
> > + unimportant, as it will be deep-copied into *STATUS.
>
> I'd just drop all mention of the pool lifetime, documenting that it is
> "unimportant" is a bit silly. Don't mention the deep-copy either,
> it's an implementation detail. In general if functions don't mention
> lifetime then it should be safe to assume that the parameters doesn't
> need to live beyond the function call.
Thanks guys, removed in r16466.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: libsvn_wc/status.c:assemble_status() doc string lies?
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> Daniel Rall <dl...@finemaltcoding.com> writes:
>> --- status.c (revision 16464)
>> +++ status.c (working copy)
>> @@ -190,11 +190,10 @@
>>
>> ENTRY may be null, for non-versioned entities. In this case, we
>> will assemble a special status structure item which implies a
>> - non-versioned thing.
>> + non-versioned thing. As non-null ENTRY will be deep copied (as
>> + opposed to stored directly), and the lifetime its pool is not
>> + important.
>>
>> - Else, ENTRY's pool must not be shorter-lived than STATUS's, since
>> - ENTRY will be stored directly, not copied.
>> -
>> PARENT_ENTRY is the entry for the parent directory of PATH, it may be
>> NULL if ENTRY is NULL or if PATH is a working copy root. The lifetime
>> of PARENT_ENTRY's pool is not important.
>
> A bit verbose, maybe just:
>
> + non-versioned thing. If non-null, ENTRY's lifetime is
> + unimportant, as it will be deep-copied into *STATUS.
I'd just drop all mention of the pool lifetime, documenting that it is
"unimportant" is a bit silly. Don't mention the deep-copy either,
it's an implementation detail. In general if functions don't mention
lifetime then it should be safe to assume that the parameters doesn't
need to live beyond the function call.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: libsvn_wc/status.c:assemble_status() doc string lies?
Posted by kf...@collab.net.
Daniel Rall <dl...@finemaltcoding.com> writes:
> --- status.c (revision 16464)
> +++ status.c (working copy)
> @@ -190,11 +190,10 @@
>
> ENTRY may be null, for non-versioned entities. In this case, we
> will assemble a special status structure item which implies a
> - non-versioned thing.
> + non-versioned thing. As non-null ENTRY will be deep copied (as
> + opposed to stored directly), and the lifetime its pool is not
> + important.
>
> - Else, ENTRY's pool must not be shorter-lived than STATUS's, since
> - ENTRY will be stored directly, not copied.
> -
> PARENT_ENTRY is the entry for the parent directory of PATH, it may be
> NULL if ENTRY is NULL or if PATH is a working copy root. The lifetime
> of PARENT_ENTRY's pool is not important.
A bit verbose, maybe just:
+ non-versioned thing. If non-null, ENTRY's lifetime is
+ unimportant, as it will be deep-copied into *STATUS.
?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: libsvn_wc/status.c:assemble_status() doc string lies?
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Tue, 04 Oct 2005, Daniel Rall wrote:
...
> However, it appears to do a deep copy of entry:
>
> stat->entry = svn_wc_entry_dup (entry, pool);
>
> From svn_wc.h:
>
> /** Return a duplicate of @a entry, allocated in @a pool. No part of the new
> * entry will be shared with @a entry.
> */
> svn_wc_entry_t *svn_wc_entry_dup (const svn_wc_entry_t *entry,
> apr_pool_t *pool);
...
* subversion/libsvn_wc/status.c
(assemble_status): Correct doc string describing the handling of the
ENTRY parameter and the lifetime of its pool.
--- status.c (revision 16464)
+++ status.c (working copy)
@@ -190,11 +190,10 @@
ENTRY may be null, for non-versioned entities. In this case, we
will assemble a special status structure item which implies a
- non-versioned thing.
+ non-versioned thing. As non-null ENTRY will be deep copied (as
+ opposed to stored directly), and the lifetime its pool is not
+ important.
- Else, ENTRY's pool must not be shorter-lived than STATUS's, since
- ENTRY will be stored directly, not copied.
-
PARENT_ENTRY is the entry for the parent directory of PATH, it may be
NULL if ENTRY is NULL or if PATH is a working copy root. The lifetime
of PARENT_ENTRY's pool is not important.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: libsvn_wc/status.c:assemble_status() doc string lies?
Posted by kf...@collab.net.
Daniel Rall <dl...@finemaltcoding.com> writes:
> assemble_status() produces a svn_wc_status2_t. Part of its input for
> assembly of that structure is a svn_wc_entry_t. About that entry, it
> claims:
>
> ENTRY may be null, for non-versioned entities. In this case, we
> will assemble a special status structure item which implies a
> non-versioned thing.
>
> Else, ENTRY's pool must not be shorter-lived than STATUS's, since
> ENTRY will be stored directly, not copied. <==== *
>
> However, it appears to do a deep copy of entry:
>
> stat->entry = svn_wc_entry_dup (entry, pool);
>
> >From svn_wc.h:
>
> /** Return a duplicate of @a entry, allocated in @a pool. No part of the new
> * entry will be shared with @a entry.
> */
> svn_wc_entry_t *svn_wc_entry_dup (const svn_wc_entry_t *entry,
> apr_pool_t *pool);
>
> It seems that it is then a lie that we care about the lifetime of entry's
> pool?
I think you're right. And the code appears to have been that way for
a very long time, it was r205 that did it. I think we can just remove
that bit from assemble_status()'s doc string. I can do it if you
want, but since you noticed, it makes sense for your name to be on the
commit. Let me know if you want me to make the tweak.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org