On Mon, Oct 21, 2013 at 7:25 AM, Branko Čibej <brane_at_wandisco.com> wrote:
> 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.
>
Committed as r1534110.
-- Stefan^2.
Received on 2013-10-21 14:00:22 CEST