On 20.10.2013 23:26, Stefan Fuhrmann wrote:
> On Fri, Oct 18, 2013 at 3:36 PM, Branko Čibej <br...@wandisco.com
> <mailto:br...@wandisco.com>> wrote:
>
>     On 16.10.2013 00:32, stef...@apache.org
>     <mailto:stef...@apache.org> wrote:
>>     Author: stefan2
>>     Date: Tue Oct 15 22:32:44 2013
>>     New Revision: 1532572
>>
>>     URL: http://svn.apache.org/r1532572
>>     Log:
>>     Add support for read-only access to svn_config_t.  In read-only mode,
>>     concurrent multi-threaded access to the same config data structure is 
>>     safe.
>
>>     +  /* Ignore write attempts to r/o configurations.
>>     +   * 
>>     +   * Since we should never try to modify r/o data, trigger an assertion
>>     +   * in debug mode.
>>     +   */
>>     +  assert(!cfg->read_only);
>>     +  if (cfg->read_only)
>>     +    return;
>>     +
>>        remove_expansions(cfg);
>
>     Please don't use assert like this. You're assuming that what
>     people like to call "release" builds are always compiled with
>     -DNDEBUG. I've always found that assumption to be naïve at best.
>
>     Instead, make the code depend on whether we're in maintainer mode
>     or not; the result will be much less ambiguous.
>
>
> From what I can see, there is no macro to test for
> (SVN_DEBUG being more or less equivalent to DEBUG).
>
> Should we introduce SVN_MAINTAINER_MODE?

I'd vote for

    #ifdef SVN_DEBUG
    SVN_ERR_ASSERT_NO_RETURN(!cfg->read_only)
    #endif

It looks like the same as "#ifndef NDEBUG", but the important difference
is that we control it independently of NDEBUG, and more importantly, we
already use it for these kinds of checks.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. br...@wandisco.com

Reply via email to