On Thu, Jul 11, 2013 at 05:18:28PM -0400, Greg Stein wrote: > On Thu, Jul 11, 2013 at 3:53 PM, Branko ??ibej <br...@wandisco.com> wrote: > > On 11.07.2013 20:08, danie...@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()?