[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1502305 - in /subversion/trunk/subversion: bindings/cxxhl/src/exception.cpp bindings/swig/python/libsvn_swig_py/swigutil_py.c libsvn_subr/error.c libsvn_subr/nls.c mod_dav_svn/mod_dav_svn.c

From: Daniel Shahaf <danielsh_at_apache.org>
Date: Thu, 11 Jul 2013 21:31:54 +0000

On Thu, Jul 11, 2013 at 05:18:28PM -0400, Greg Stein wrote:
> On Thu, Jul 11, 2013 at 3:53 PM, Branko Čibej <brane_at_wandisco.com> wrote:
> > On 11.07.2013 20:08, danielsh_at_apache.org wrote:
> >> Author: danielsh
> >> Date: Thu Jul 11 18:08:23 2013
> >> New Revision: 1502305
> >>
> >> URL: http://svn.apache.org/r1502305
> >> Log:
> >> Use svn_pool_create() instead of apr_pool_create().
> >>
> >> Presently, this means that if an apr_pool_create() fails, abort_fn() will be
> >> called. None of those plafces check for NULL results from the allocator,
> >> so the net effect is changing a NULL dereference to calling our pool.c
> >> function abort_on_pool_failure() (which is marginally better).
> >>
> >> * subversion/bindings/cxxhl/src/exception.cpp
> >> (Error::compile_messages):
> >
> > This change is wrong, please revert it. I agree the code needs to check
> > for the null return, however, replacing the current mode with an abort
> > is not "marginally better", it's completely wrong.
>
> How is a NULL dereference better? We can't catch that in some way, can we?
>
> Or will you simply be adding the NULL checks?

exception.cpp does not allocate anything; it just passes that pool to
libsvn_subr/utf.c functions. Perhaps Brane intends to catch the apr_status_t
return value of apr_pool_create()?
Received on 2013-07-11 23:31:59 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.