Re: svn commit: r1329234 - in /subversion/trunk: ./ subversion/libsvn_delta/compat.c

On Mon, Apr 23, 2012 at 8:53 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: hwright_at_apache.org [mailto:hwright_at_apache.org]
>> Sent: maandag 23 april 2012 15:44
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1329234 - in /subversion/trunk: ./
>> subversion/libsvn_delta/compat.c
>>
>> Author: hwright
>> Date: Mon Apr 23 13:43:33 2012
>> New Revision: 1329234
>>
>> URL: http://svn.apache.org/viewvc?rev=1329234&view=rev
>> Log:
>> Merge r1325914 from the ev2-export branch.
>>
>> Modified:
>>     subversion/trunk/   (props changed)
>>     subversion/trunk/subversion/libsvn_delta/compat.c
>>
>> Propchange: subversion/trunk/
>> ------------------------------------------------------------------------------
>>   Merged /subversion/branches/ev2-export:r1325914
>>
>> Modified: subversion/trunk/subversion/libsvn_delta/compat.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/compa
>> t.c?rev=1329234&r1=1329233&r2=1329234&view=diff
>> =================================================================
>> =============
>> --- subversion/trunk/subversion/libsvn_delta/compat.c (original)
>> +++ subversion/trunk/subversion/libsvn_delta/compat.c Mon Apr 23 13:43:33
>> 2012
>> @@ -1088,6 +1088,11 @@ svn_delta__delta_from_editor(const svn_d
>>      };
>>    struct ev2_edit_baton *eb = apr_pcalloc(pool, sizeof(*eb));
>>
>> +  if (!base_relpath)
>> +    base_relpath = "";
>> +  else if (base_relpath[0] == '/')
>> +    base_relpath += 1;
>> +
>
> Why do you allow invalid relpaths?
>
> I think it is the callers task to pass valid relpaths, so you could just use assert(svn_relpath_is_canonical(base_relpath)); for a debug only check. (or SVN_ERR_ASSERT() if you also want to slow down release versions)

Some callers don't have a relpath to pass, which is the reason for the
first check. I think you are correct, though that we can modify the
code to require callers provide a valid relpath, if they provide one
at all. It might take me a while to get there, though.

As an aside, I thought SVN_ERR_ASSERT() was compiled out of production
builds. Perhaps not?

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/