RE: svn commit: r888102 - /subversion/trunk/subversion/libsvn_subr/dirent_uri.c

Bert Huijben wrote:
> > From: julianfoad_at_apache.org [mailto:julianfoad_at_apache.org]
[...]
> > * subversion/libsvn_subr/dirent_uri.c
> > (svn_dirent_get_absolute): Assert that the input is not a URL, because
> > that mistake was made several times recently and new calls to this
> > function are being added frequently.
>
> I think we should use an assert(svn_dirent_is_canonical(relative,
> pool)); here, like in the other dirent functions.

Consistency is good. I did look up and down the file for any existing
examples, and the nearest few dirent functions before and after that one
don't have any assertions in them so I didn't see that.

> This verifies a bit more than just if it is not a url-like path and it

But is_canonical() doesn't verify that it isn't a URL, does it? Ah, I
suppose it does, because a URL contains "//" which would be
non-canonical in a dirent. OK, that would be good. Feel free to change
it.

> also disables the check in release mode as we do in most path
> handling.

I don't understand if that's meant to be an argument for using assert().
If assert() is disabled in release mode and SVN_ERR_ASSERT() isn't, that
sounds like an inconsistency in the build system, not a reason to avoid
using SVN_ERR_ASSERT.

- Julian
Received on 2009-12-09 00:20:30 CET