Login | Register
My pages Projects Community openCollabNet

Discussions > dev [DISABLED] > Re: svn commit: r37590 - trunk/subversion/libsvn_wc

subversion
Discussion topic

Back to topic list

Re: svn commit: r37590 - trunk/subversion/libsvn_wc

Author hwright
Full name Hyrum K. Wright
Date 2009-05-06 05:35:44 PDT
Message On May 6, 2009, at 3:43 AM, Bert Huijben wrote:

>> -----Original Message-----
>> From: Hyrum K. Wright [mailto:hyrum at hyrumwright dot org]
>> Sent: dinsdag 5 mei 2009 22:53
>> To: Greg Stein
>> Cc: dev at subversion dot tigris dot org
>> Subject: Re: svn commit: r37590 - trunk/subversion/libsvn_wc
>>
>> On May 5, 2009, at 3:44 PM, Greg Stein wrote:
>>
>>> On Tue, May 5, 2009 at 22:19, Hyrum K. Wright
>>> <hyrum at hyrumwright dot org> wrote:
>>>> ...
>>>> +++ trunk/subversion/lib​svn_wc/adm_ops.c Tue May 5 13:19:56
>>>> 2009 (r37590)
>>>> ...
>>>> @@ -2445,8 +2451,12 @@ svn_wc_remove_from_r​evision_control(svn_​
>>>> svn_error_t *err;
>>>> svn_boolean_t is_file;
>>>> svn_boolean_t left_something = FALSE;
>>>> + svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
>>>> const char *full_path = apr_pstrdup(pool,
>>>>
>>>> svn_wc_adm_access_pa​th(adm_access));
>>>
>>> I know this isn't part of your change, but this seems dumb. By
>>> definition, the baton's path will last longer than this function. Is
>>> there a reason to make a copy of this string?
>>
>> I don't know / haven't looked at it too deeply.
>>
>>>> + const char *local_abspath;
>>>> +
>>>> + SVN_ERR(svn_path_get​_absolute(&local​_abspath, full_path, pool));
>>>>
>>>> /* Check cancellation here, so recursive calls get checked early.
>>>> */
>>>> if (cancel_func)
>>>> @@ -2460,10 +2470,9 @@ svn_wc_remove_from_r​evision_control(svn_​
>>>> svn_node_kind_t kind;
>>>> svn_boolean_t wc_special, local_special;
>>>> svn_boolean_t text_modified_p;
>>>> - svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
>>>> - const char *local_abspath;
>>>>
>>>> full_path = svn_dirent_join(full_path, name, pool);
>>>> + SVN_ERR(svn_path_get​_absolute(&local​_abspath, full_path,
>>>> pool));
>>>
>>> You did this above.
>>
>> Sure, but the value for full_path has changed, so this will yield a
>> different result.
>
> But an entryname/basename joined to an absolute path will always
> return an
> absolute path, so this step is not necessary.

Understood, but I don't see any place that we guarantee full_path is
an absolute path to begin with. It may be an absolute path, or it may
be a path relative to the cwd or maybe to the working copy root.
That's one of the problems with the current working copy: we keep
tossing around these paths, but don't really say what kind they are.
(And in many places, we aren't too strict about what we accept.)

> The only case where joining a basename to an absolute path doesn't
> give you
> an absolute path is when the basename is '.' or '..', but these
> values are
> never valid as entrynames.

-Hyrum

« Previous message in topic | 4 of 4 | Next message in topic »

Messages

Show all messages in topic

Re: svn commit: r37590 - trunk/subversion/libsvn_wc gstein Greg Stein 2009-05-05 13:44:19 PDT
     Re: svn commit: r37590 - trunk/subversion/libsvn_wc hwright Hyrum K. Wright 2009-05-05 13:52:39 PDT
         RE: svn commit: r37590 - trunk/subversion/libsvn_wc rhuijben Bert Huijben 2009-05-06 01:43:29 PDT
             Re: svn commit: r37590 - trunk/subversion/libsvn_wc hwright Hyrum K. Wright 2009-05-06 05:35:44 PDT
Messages per page: