Re: svn commit: r1402907 - /apr/apr/trunk/buckets/apr_buckets_alloc.c

[Stefan Fritsch]

On Mon, 5 Nov 2012, Ruediger Pluem wrote:
s...@apache.org wrote:
Author: sf
Date: Sat Oct 27 22:56:41 2012
New Revision: 1402907
URL: http://svn.apache.org/viewvc?rev=1402907&view=rev
Log:
If out of mem, abort instead of crashing. Use the pool's abort function
if it has one.

Modified:
    apr/apr/trunk/buckets/apr_buckets_alloc.c

Modified: apr/apr/trunk/buckets/apr_buckets_alloc.c
URL: 
http://svn.apache.org/viewvc/apr/apr/trunk/buckets/apr_buckets_alloc.c?rev=1402907&r1=1402906&r2=1402907&view=diff
==============================================================================
--- apr/apr/trunk/buckets/apr_buckets_alloc.c (original)
+++ apr/apr/trunk/buckets/apr_buckets_alloc.c Sat Oct 27 22:56:41 2012
@@ -57,11 +57,14 @@ APR_DECLARE_NONSTD(apr_bucket_alloc_t *)
     apr_allocator_t *allocator;
     apr_bucket_alloc_t *list;

-    if (apr_allocator_create(&allocator) != APR_SUCCESS) {
-        abort();
+    if (apr_allocator_create(&allocator) != APR_SUCCESS
+        || (list = apr_bucket_alloc_create_ex(allocator)) == NULL) {
+        apr_abortfunc_t fn = apr_pool_abort_get(p);
+        if (fn)
+            (fn)(APR_ENOMEM);

Are we sure that fn never returns? If it does list further down below is not 
initialized and further things could go
wrong. Shouldn't we return NULL here to be save?

Considering that the old code called abort, just calling abort() even if 
fn returns seems to be more consistent. Committed in r1406088

+        else
+            abort();
     }
-
-    list = apr_bucket_alloc_create_ex(allocator);
     list->pool = p;
     apr_pool_cleanup_register(list->pool, list, alloc_cleanup,
                               apr_pool_cleanup_null);