You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2009/12/07 20:33:31 UTC
svn commit: r888102 - /subversion/trunk/subversion/libsvn_subr/dirent_uri.c
Author: julianfoad
Date: Mon Dec 7 19:33:29 2009
New Revision: 888102
URL: http://svn.apache.org/viewvc?rev=888102&view=rev
Log:
* 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.
Modified:
subversion/trunk/subversion/libsvn_subr/dirent_uri.c
Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=888102&r1=888101&r2=888102&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Mon Dec 7 19:33:29 2009
@@ -1540,6 +1540,8 @@
apr_status_t apr_err;
const char *path_apr;
+ SVN_ERR_ASSERT(! svn_path_is_url(relative));
+
/* Merge the current working directory with the relative dirent. */
SVN_ERR(svn_path_cstring_from_utf8(&path_apr, relative, pool));
RE: svn commit: r888102 -
/subversion/trunk/subversion/libsvn_subr/dirent_uri.c
Posted by Julian Foad <ju...@btopenworld.com>.
Bert Huijben wrote:
> > From: julianfoad@apache.org [mailto:julianfoad@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
RE: svn commit: r888102 - /subversion/trunk/subversion/libsvn_subr/dirent_uri.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: julianfoad@apache.org [mailto:julianfoad@apache.org]
> Sent: maandag 7 december 2009 20:34
> To: commits@subversion.apache.org
> Subject: svn commit: r888102 -
> /subversion/trunk/subversion/libsvn_subr/dirent_uri.c
>
> Author: julianfoad
> Date: Mon Dec 7 19:33:29 2009
> New Revision: 888102
>
> URL: http://svn.apache.org/viewvc?rev=888102&view=rev
> Log:
> * 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.
This verifies a bit more than just if it is not a url-like path and it also disables the check in release mode as we do in most path handling.
Bert
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/dirent_uri.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dir
> ent_uri.c?rev=888102&r1=888101&r2=888102&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Mon Dec 7
> 19:33:29 2009
> @@ -1540,6 +1540,8 @@
> apr_status_t apr_err;
> const char *path_apr;
>
> + SVN_ERR_ASSERT(! svn_path_is_url(relative));
> +
> /* Merge the current working directory with the relative dirent. */
> SVN_ERR(svn_path_cstring_from_utf8(&path_apr, relative, pool));
>
>
RE: svn commit: r888102 - /subversion/trunk/subversion/libsvn_subr/dirent_uri.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: julianfoad@apache.org [mailto:julianfoad@apache.org]
> Sent: maandag 7 december 2009 20:34
> To: commits@subversion.apache.org
> Subject: svn commit: r888102 -
> /subversion/trunk/subversion/libsvn_subr/dirent_uri.c
>
> Author: julianfoad
> Date: Mon Dec 7 19:33:29 2009
> New Revision: 888102
>
> URL: http://svn.apache.org/viewvc?rev=888102&view=rev
> Log:
> * 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.
This verifies a bit more than just if it is not a url-like path and it also disables the check in release mode as we do in most path handling.
Bert
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/dirent_uri.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dir
> ent_uri.c?rev=888102&r1=888101&r2=888102&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Mon Dec 7
> 19:33:29 2009
> @@ -1540,6 +1540,8 @@
> apr_status_t apr_err;
> const char *path_apr;
>
> + SVN_ERR_ASSERT(! svn_path_is_url(relative));
> +
> /* Merge the current working directory with the relative dirent. */
> SVN_ERR(svn_path_cstring_from_utf8(&path_apr, relative, pool));
>
>