On Fri, Jun 13, 2008 at 01:07:00PM -0400, Karl Fogel wrote:
> Stefan K??ng <tortoisesvn_at_gmail.com> writes:
> >> * assert should only be used for "this can't happen" sanity checks
> >
> > I disagree: assert should *never* be used, unless it's enclosed in
> > #ifdef _DEBUG statements.
>
> assert() is a no-op if NDEBUG is defined. Is NDEBUG defined in our
> production code? If not, shouldn't it be?
AFAIK, we do build without -NDEBUG by default.
We used to never define -DNDEBUG. I changed our configure script
to define -DNDEBUG when --disabled-debug is passed, in this commit:
------------------------------------------------------------------------
r31121 | stsp | 2008-05-11 11:44:08 +0200 (Sun, 11 May 2008) | 37 lines
Remove assertions and other debugging code ifdef'd with
NDEBUG if debugging is not enabled at compile time.
So far, this code was always compiled in, even in releases.
In path handling functions, assert that paths supplied
are canonical when debugging is enabled. This isn't new,
some functions already did so before this change, but now
it is consistent, and together with the above change,
doing this now actually makes sense.
* subversion/libsvn_subr/path.c
(is_canonical): Document, extend a little, and rewrite partly
for obviousness. This function has been around for much longer
than svn_path_is_canonical, and tries to provide the same
functionality, but does not quite match up. Confine its use
to functions that don't have a pool around.
(svn_path_join, svn_path_join_many, svn_path_add_component,
svn_path_remove_component, svn_path_dirname, svn_path_basename,
svn_path_is_empty, svn_path_compare_paths, svn_path_is_child,
svn_path_decompose, svn_path_is_single_path_component,
svn_path_split_if_file): When debugging is enabled, assert that
paths supplied are canonical, using svn_path_is_canonical if possible,
otherwise using is_canonical. The API requires all paths supplied to
be canonical. Some of these functions have already done the same
assertion with is_canonical before, but for a few the assertion was
commented out because the call to strlen involved was considered
expensive ...eh? This is debugging code, it's not supposed to
perform well, it's a safety net.
* subversion/tests/libsvn_subr/path-test.c
(test_is_single_path_component): Don't use a non-canonical path
in test set, the API being tested here does not support this.
This was only working because the assertion in the function
being tested was commented out.
* configure.ac: Compile with -DNDEBUG if debugging is off.
------------------------------------------------------------------------
Stefan
- application/pgp-signature attachment: stored
Received on 2008-06-13 21:18:34 CEST