Re: svn commit: rev 5521 [...]

> > Wouldn't be fixing the docstring instead be more handy (see below)?
> > I.e. having an empty string meaning "no common ancestor". This would
> > allow to use the result directly in a function like apr_pstrcat().
[...]
> > With the suggested change to svn_path_get_longest_ancestor() this

That should read get_longest_path_ancestor (the function the suggested
change above is about).

> > wouldn't work anymore. If it returns an empty string, either a check
> > for that is needed or the docstring must be changed. Hm. In the former
> > case we are back to the if-condition for the return value and have
> > only effectively moved down from above. On the other hand, most
> > consumers did not check for NULL values until before you changed it
>
> i don't understand what you're saying here.

Sorry. I'll try again.

Short version: If you change both, svn_path_get_longest_ancestor and
get_longest_path_ancestor to return empty strings instead of NULL,
it's not important anymore.

Long version: As you know, I suggested to simplify
get_longest_path_ancestor by returning an empty string instead of NULL
in case of no ancestor. That gets rid of an if-condition before using
the return value for assembling new strings.

Later down, you make use of get_longest_path_ancestor's original
interface by directly passing through the its return value to the
caller. So if the interface of svn_path_get_longest_ancestor (the
function we are in) stays the same, you'd to add an if-condition that
checks if get_longest_path_ancestor gives an empty string and return a
NULL instead.

That would mean that, effictively, my suggestion did just move around
the condition instead of making in superfluous. Except, if we
propagate the change and also make the interface of
svn_path_get_longest_ancestor return an empty string instead of NULL.
Which was the behaviour (despite the doc) of most callers of
svn_path_get_longest_ancestor before your change anyhow.

In summary, I simply suggest to make svn_path_get_longest_ancestor
return empty strings, too, because else there is not much benefit in
changing get_longest_path_ancestor.

   Benjamin.