[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] issue #3620: svn add ^/ triggers assertion failure

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 23 Apr 2010 12:39:46 -0400

On Fri, Apr 23, 2010 at 11:58, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Uwe Stuehler wrote:
>> [[[
>> Fix issue #3620: svn add ^/ triggers assertion failure
>>
>> * subversion/svn/add-cmd.c:
>>   (svn_cl__add): Raise an error if a target argument isn't a working
>>   copy path before calling svn_client_add4(), which would trigger an
>>   assertion.
>>
>> Found by: stsp
>> Patch by: Uwe Stuehler (subversion-lists_at_bsdx.de)
>> ]]]
>
> Uwe,
>
> Thanks again for your patch.  I took some time to look at it just now.  The
> patch, as patches go, is fine.  But the approach isn't the best one.  Here
> are the notes I just recorded in issue #3620:
>
> ----------------------------------------------------------------------------
> 'svn st ^/' suffers from the same thing.
>
> Subversion 1.6 handles these cases gracefully.  Note the use of the
> svn_cl__try() wrapper arounds the client's calls to svn_client_status5() and
> svn_client_add4() -- it clearly is expecting those APIs to return a
> particular error code when asked to operate on paths that aren't working
> copies.  The fix for this issue needs to preserve those behaviors (which
> means it must live behind the libsvn_client API).
>
> I'm not confident, but I suspect that the root cause is the use of
> svn_dirent_get_absolute() all over the place for WC-NG-ness.  That function
> asserts (ultimately) that its input isn't a URL.  We need to either make it
> return a graceful error in those conditions, or do this kind of checking in
> its callers.
> ----------------------------------------------------------------------------
>
> We need to figure out the right approach before moving on.

I don't think this is rocket science. My comment in the issue:

----------------------------------------------------------------------------
I think that svn_client_add4 should check if a URL is passed and
return an error. Simple as that. Then we
can proceed with the svn_dirent_get_absolute call.

I don't really see the point about prior error returns and svn_cl__try
and stuff. It is clear that a bad input
was provided to svn_client_add4, and it should return an error stating such.
----------------------------------------------------------------------------

Basically, put Uwe's patch right into svn_client_add4, rather than in
the cmdline tool.

Cheers,
-g
Received on 2010-04-23 18:40:14 CEST

This is an archived mail posted to the Subversion Dev mailing list.