On 20.10.2013 23:26, Stefan Fuhrmann wrote:
> On Fri, Oct 18, 2013 at 3:36 PM, Branko Čibej <brane_at_wandisco.com
> <mailto:brane_at_wandisco.com>> wrote:
>
> On 16.10.2013 00:32, stefan2_at_apache.org
> <mailto:stefan2_at_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. brane_at_wandisco.com
Received on 2013-10-21 07:26:31 CEST