On Thu, 2011-03-03 at 22:48 +0530, Noorul Islam K M wrote:
> Julian Foad <julian.f...@wandisco.com> writes:
> 
> > On Wed, 2011-03-02, Noorul Islam K M wrote:
> >
> >> Please find attached patch for issue 3826. All tests pass using 'make
> >> check' 
> > [...]
> >> Index: subversion/svn/diff-cmd.c
> >> ===================================================================
> >> --- subversion/svn/diff-cmd.c      (revision 1076214)
> >> +++ subversion/svn/diff-cmd.c      (working copy)
> >> @@ -324,8 +324,11 @@
> >>              return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> >>                                       _("Path '%s' not relative to base 
> >> URLs"),
> >>                                       path);
> >> +          if (! svn_dirent_is_absolute(path))
> >> +            path = svn_relpath_canonicalize(path, iterpool);
> >> +          else
> >> +            path = svn_dirent_canonicalize(path, iterpool);

Your new code here expects that 'path' could be either an absolute local
path (know as a 'dirent'), or a 'relpath' ...

> >>  
> >> -          path = svn_relpath_canonicalize(path, iterpool);
> >>            if (svn_path_is_url(old_target))
> >>              target1 = svn_path_url_add_component2(old_target, path, 
> >> iterpool);

... but here (if old_target is a URL) the code requires that 'path' is a
relpath.  So what happens if 'path' is a 'dirent'?  Does this
'url_add_component' line get executed (and if so it will produce wrong
results), or does it not get executed (and if not, why not)?

- Julian


> > This doesn't look quite right.  Here, "path" must be a relpath ...
> >
> >>            else
> >                target1 = svn_dirent_join(old_target, path, iterpool);
> >
> > ... even though here it can be either a relpath or an absolute dirent.
> >
> 
> I could not understand what you are trying to convey. Can you please
> give more information?
> 
> Thanks and Regards
> Noorul


Reply via email to