-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Julian Foad wrote: > Oh yes, so you did. However, that patch you initially proposed there > passed a part of the URL to svn_dirent_get_absolute(), which is not > right either.
Yes, then I had a different patch altogether. >>> I suggest adding >>> >>> SVN_ERR_ASSERT(!svn_path_is_url(relative)); >>> >>> inside svn_dirent_get_absolute(), at least for debugging, as we should >>> never be calling it on any path that looks like a URL. When I tried >>> doing that, almost all tests failed, presumably because some universal >>> operation is calling it on a URL. >> In cases of handling both URLs and wc paths, there's a check made on >> the type of the path based on which the desired dirent/URI method is >> invoked right(except for the rare case above)? > > Yes: the case above is a bug, and in all non-buggy cases we only call > svn_dirent_get_absolute() on a dirent. > >> Indeed the 'assert' >> suggested above is useful for debugging, but if we're to check the >> type of the path before making a call to this method, which confirms >> it's not a URL then, won't the 'assert' here become obsolete? > > "Assertions are forever." (A quote from the book "Writing Solid Code".) > The purpose of assertions is to catch programming errors (bugs). They > don't become obsolete when we have eliminated all known bugs, they stay > there to catch any future bugs or any bugs that haven't so far shown up. +1. Was about to update my thought on this. >> Also for the above case, may I suggest the following change? >> [[[ >> if(!svn_path_is_url(source) >> SVN_ERR(svn_dirent_get_absolute(&source_abspath, source, pool)); >> else >> source_abspath = source; >> ]]] > > It is very misleading to put a URL into a variable called > "source_abspath". If the variable needs to serve a dual purpose, please > rename it to something like "source_abspath_or_url". Sure, suggested on seeing the code snip. I'll look onto other such ..dirent_absolute() calls made on a URL and send a tested patch. - -- Thanks & Regards, Kannan -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEVAwUBSxAhUHlTqcY7ytmIAQKRBwgAhP84alZ1fbPz4YlCbnwIioL14UmPLRfD xhYv5nvu1DP+LuHLCsp+ebm56/X7hE2TnhXRbji+1vKPml93f5RAcnMMLw+3kOmW ZW4fEeSdUjVT+PASniGypdCtUIci3da6JdNYd6JXa2a3o936KGwjtIQxTga/y4XP GM4YRvyQjchf3bFCtA7NpQ7xKke44AJ2OjolvoxtHpXBj5wDd8OJhXZVJqCeLr0E 6DSZhnX7zvFVchInCvfc1bbaLiTmWFpelYdDdh8sDV660PMweZ0p0aT9N4Pjm4g4 OX6IsWc66e74zlkK51nlRs2u/4HCkEFWl3OiuZt8CV6g4y0Te1Xm/g== =kLPp -----END PGP SIGNATURE-----
