You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by sf...@apache.org on 2012/10/28 00:56:41 UTC

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

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);
+        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);



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

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Nov 5, 2012, at 11:11 AM, Ruediger Pluem <rp...@apache.org> wrote:

>> 
>> -    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?
> 

+1

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

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Mon, 5 Nov 2012, Ruediger Pluem wrote:
> sf@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);
>>

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

Posted by Ruediger Pluem <rp...@apache.org>.

sf@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?


> +        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);
> 
> 
> 


Regards

RĂ¼diger