Re: crash fetching status

Stefan Küng wrote:

> On Thu, Dec 29, 2011 at 16:57, Stefan Küng wrote:
>> On 29.12.2011 16:43, Hyrum K Wright wrote:
>>> On Wed, Dec 28, 2011 at 1:01 AM, Stefan Küng wrote:

[...]

>>> I looked at similar places elsewhere, and they seemed to follow a
>>> pattern.  The following patch is in a similar vein:
>>>
>>> [[[
>>> Index: subversion/libsvn_wc/status.c
>>> ===================================================================
>>> -  f->repos_relpath = svn_relpath_join(find_dir_repos_relpath(pb, pool),
>>> -                                      f->name, pool);
>>> +
>>> +  dir_repos_relpath = find_dir_repos_relpath(pb, pool);
>>> +  if (dir_repos_relpath)
>>> +    f->repos_relpath = svn_relpath_join(dir_repos_relpath, f->name,
>>> pool);
>>> +  else
>>> +    f->repos_relpath = apr_pstrdup(pool, f->name);
>>> +
>>>
>>> All the tests pass, though I'm not sure if there are other correctness
>>> issues here or not, so I haven't committed this.
>>
>> Just a thought:
>> svn_relpath_join() joins two path/url parts. If one is an empty string, it
>> still works. But it crashes when one of the parts is NULL instead of an
>> empty string.

Yes, because NULL is not a valid relpath.  The 'relpath' concept is well defined (in svn_dirent_uri.h).  The representation of an empty relpath is the empty string.

The real problem here is that find_dir_repos_relpath() can return NULL whereas its callers (including but not limited to this one) expect a non-null value.

So the real solution is either to change find_dir_repos_relpath() so it always returns a valid 'relpath', or to change its callers to code around the fact that it can return NULL.

Does it return NULL to mean the empty relpath or for some other reason?

>> Maybe just rewrite svn_relpath_join() so that NULL is treated as an empty
>> string?

No.  Don't make an exception.

- Julian
Received on 2012-01-03 11:24:33 CET