Bert Huijben wrote on Thu, May 19, 2011 at 17:41:13 +0200:
>
>
> > -----Original Message-----
> > From: Daniel Shahaf [mailto:danielsh_at_elego.de]
> > Sent: donderdag 19 mei 2011 17:36
> > To: dev_at_subversion.apache.org
> > Cc: Daniel Shahaf
> > Subject: Re: Locking and errors (and deserialization) in inprocess-cache
> >
> > Actually, the following might suffice for now:
> >
> > Index: subversion/libsvn_subr/cache-inprocess.c
> > ==========================================================
> > =========
> > --- subversion/libsvn_subr/cache-inprocess.c (revision 1124903)
> > +++ subversion/libsvn_subr/cache-inprocess.c (working copy)
> > @@ -148,19 +148,19 @@ insert_page(inprocess_cache_t *cache,
> >
> > /* If PAGE is in the circularly linked list (eg, its NEXT isn't NULL),
> > * move it to the front of the list. */
> > -static svn_error_t *
> > +static void
> > move_page_to_front(inprocess_cache_t *cache,
> > struct cache_page *page)
> > {
> > - SVN_ERR_ASSERT(page != cache->sentinel);
> > + SVN_ERR_ASSERT_NO_RETURN(page != cache->sentinel);
>
> But this would crash the application/server if it fails, while the other
> would return an error if a malfunction handler is installed.
>
> So, "no". This is not a valid alternative.
>
> The original assert() before your original patch is a valid alternative as
> that won't crash the application.
>
If we have assert() then a non-debug server would be accessing a cache
whose contents are undefined, which may result in corrupt repositories
(since the cache is used for FS contents). I'd rather crash.
> Bert
>
Received on 2011-05-19 18:33:46 CEST