svn commit: r1712306 - /serf/trunk/test/test_buckets.c

2015-11-03 Thread ivan 
Author: ivan
Date: Tue Nov  3 15:08:49 2015
New Revision: 1712306

URL: http://svn.apache.org/viewvc?rev=1712306=rev
Log:
* test/test_buckets.c
  (test_aggregate_bucket_readline): Remove unused code.

Modified:
serf/trunk/test/test_buckets.c

Modified: serf/trunk/test/test_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1712306=1712305=1712306=diff
==
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Tue Nov  3 15:08:49 2015
@@ -768,7 +768,6 @@ static void test_aggregate_bucket_readli
 bkt = SERF_BUCKET_SIMPLE_STRING_LEN(BODY+22, strlen(BODY)-22, alloc);
 serf_bucket_aggregate_append(aggbkt, bkt); /* 2nd and 3rd line */
 
-bkt = SERF_BUCKET_SIMPLE_STRING(BODY, alloc);
 readlines_and_check_bucket(tc, aggbkt, SERF_NEWLINE_CRLF, BODY, 3);
 
 /* Test 2: start with empty bucket */
@@ -781,7 +780,6 @@ static void test_aggregate_bucket_readli
 bkt = SERF_BUCKET_SIMPLE_STRING_LEN(BODY+22, strlen(BODY)-22, alloc);
 serf_bucket_aggregate_append(aggbkt, bkt); /* 2nd and 3rd line */
 
-bkt = SERF_BUCKET_SIMPLE_STRING(BODY, alloc);
 readlines_and_check_bucket(tc, aggbkt, SERF_NEWLINE_CRLF, BODY, 3);
 }

svn commit: r1712307 - /serf/trunk/buckets/response_buckets.c

2015-11-03 Thread ivan 
Author: ivan
Date: Tue Nov  3 15:11:57 2015
New Revision: 1712307

URL: http://svn.apache.org/viewvc?rev=1712307=rev
Log:
Fix memory leak in serf_response_full_become_aggregate().

* buckets/response_buckets.c
  (serf_response_full_become_aggregate): Destroy CTX->BODY bucket if any
   and free memory allocated for status line reason.

Modified:
serf/trunk/buckets/response_buckets.c

Modified: serf/trunk/buckets/response_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/response_buckets.c?rev=1712307=1712306=1712307=diff
==
--- serf/trunk/buckets/response_buckets.c (original)
+++ serf/trunk/buckets/response_buckets.c Tue Nov  3 15:11:57 2015
@@ -555,6 +555,9 @@ apr_status_t serf_response_full_become_a
 serf_bucket_aggregate_append(bucket, ctx->headers);
 serf_bucket_aggregate_append(bucket, ctx->stream);
 
+if (ctx->body != NULL)
+serf_bucket_destroy(ctx->body);
+serf_bucket_mem_free(bucket->allocator, (void*)ctx->sl.reason);
 serf_bucket_mem_free(bucket->allocator, ctx);
 
 return APR_SUCCESS;

buildbot success in ASF Buildbot on serf-x64-macosx

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder serf-x64-macosx while 
building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx/builds/201

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712307
Blamelist: ivan

Build succeeded!

Sincerely,
 -The Buildbot

buildbot failure in ASF Buildbot on serf-x64-macosx

2015-11-03 Thread buildbot 
The Buildbot has detected a new failure on builder serf-x64-macosx while 
building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx/builds/200

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712306
Blamelist: ivan

BUILD FAILED: failed Test

Sincerely,
 -The Buildbot

Re: svn commit: r1712270 - in /serf/trunk: buckets/allocator.c buckets/buckets.c test/test_util.c

2015-11-03 Thread Greg Stein 
On Tue, Nov 3, 2015 at 6:12 AM,  wrote:
>...

> +++ serf/trunk/buckets/allocator.c Tue Nov  3 12:12:00 2015
> @@ -27,6 +27,8 @@
>  #include "serf.h"
>  #include "serf_bucket_util.h"
>
> +#include "serf_private.h"
>

Is this really needed? The changes don't seem to require exposure to the
private header.

>...

Cheers,
-g

svn commit: r1712271 - /serf/trunk/buckets/buckets.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Tue Nov  3 12:15:06 2015
New Revision: 1712271

URL: http://svn.apache.org/viewvc?rev=1712271=rev
Log:
Remove accidental garbage committed in r1712270.

* buckets/buckets.c
  (it): Remove.

Modified:
serf/trunk/buckets/buckets.c

Modified: serf/trunk/buckets/buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/buckets.c?rev=1712271=1712270=1712271=diff
==
--- serf/trunk/buckets/buckets.c (original)
+++ serf/trunk/buckets/buckets.c Tue Nov  3 12:15:06 2015
@@ -182,7 +182,7 @@ apr_status_t serf_default_ignore_config(
 {
 return APR_SUCCESS;
 }
-it
+
 /* Fallback type definition to return for buckets that don't implement
a specific version of the bucket spec */
 static const serf_bucket_type_t fallback_bucket_type =

buildbot success in ASF Buildbot on serf-windows

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder serf-windows while 
building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-windows/builds/212

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-w2k3-ra

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712271
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

buildbot success in ASF Buildbot on serf-x64-macosx

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder serf-x64-macosx while 
building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx/builds/198

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712271
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

buildbot success in ASF Buildbot on serf-sparc64-solaris

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder serf-sparc64-solaris 
while building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-sparc64-solaris/builds/177

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-sparc-solaris

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712271
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

buildbot success in ASF Buildbot on serf-x64-macosx-default-openssl

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder 
serf-x64-macosx-default-openssl while building ASF Buildbot. Full details are 
available at:
http://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/184

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712271
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

buildbot success in ASF Buildbot on serf-x64-macosx-apr1.5

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder serf-x64-macosx-apr1.5 
while building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx-apr1.5/builds/181

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712271
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

svn propchange: r1712270 - svn:log

2015-11-03 Thread rhuijben 
Author: rhuijben
Revision: 1712270
Modified property: svn:log

Modified: svn:log at Tue Nov  3 12:16:22 2015
--
--- svn:log (original)
+++ svn:log Tue Nov  3 12:16:22 2015
@@ -5,5 +5,8 @@ Fix some testsuite issues when running t
   (serf_bucket_allocator_create): When creating an allocator, set maximum
 amount of free memory to keep.
 
+* buckets/buckets.c
+  (it): Added accidental garbage. [Removed in rr1712271]
+
 * test/test_util.c
   (test_setup): Resolve segfault when running with pool debugging.

buildbot success in ASF Buildbot on serf-x64-macosx-default-openssl

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder 
serf-x64-macosx-default-openssl while building ASF Buildbot. Full details are 
available at:
http://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/182

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712264
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

buildbot failure in ASF Buildbot on serf-x64-macosx

2015-11-03 Thread buildbot 
The Buildbot has detected a new failure on builder serf-x64-macosx while 
building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx/builds/197

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712270
Blamelist: rhuijben

BUILD FAILED: failed Build

Sincerely,
 -The Buildbot

buildbot failure in ASF Buildbot on serf-x64-macosx-apr2.0-dev

2015-11-03 Thread buildbot 
The Buildbot has detected a new failure on builder serf-x64-macosx-apr2.0-dev 
while building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/175

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712270
Blamelist: rhuijben

BUILD FAILED: failed Build

Sincerely,
 -The Buildbot

svn commit: r1712312 - in /serf/trunk/test: test_buckets.c test_serf.h test_util.c

2015-11-03 Thread ivan 
Author: ivan
Date: Tue Nov  3 15:46:37 2015
New Revision: 1712312

URL: http://svn.apache.org/viewvc?rev=1712312=rev
Log:
Improve diagnostic of unfreed memory in bucket allocator in bucket tests.

* test/test_serf.h
  (test__create_bucket_allocator): New function declaration.

* test/test_util.c
  (bucket_unfreed_memory_cb): New. Callback for serf_bucket_allocator_create()
   to print unfreed memory blocks prefixed with test name.
  (test__create_bucket_allocator): New. Create bucket allocator with custom
   unfreed memory callback to print unfreed memory in test.

* test/test_buckets.c
  (*): Use test__create_bucket_allocator() instead of
   serf_bucket_allocator_create() to create bucket allocator.

Modified:
serf/trunk/test/test_buckets.c
serf/trunk/test/test_serf.h
serf/trunk/test/test_util.c

Modified: serf/trunk/test/test_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1712312=1712311=1712312=diff
==
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Tue Nov  3 15:46:37 2015
@@ -196,8 +196,7 @@ static void test_simple_bucket_readline(
 apr_size_t len;
 const char *body;
 
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 
 bkt = SERF_BUCKET_SIMPLE_STRING(
 "line1" CRLF
@@ -282,8 +281,7 @@ static void test_response_bucket_read(Cu
 test_baton_t *tb = tc->testBaton;
 serf_bucket_t *bkt, *tmp;
 
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 
 tmp = SERF_BUCKET_SIMPLE_STRING(
 "HTTP/1.1 200 OK" CRLF
@@ -304,8 +302,7 @@ static void test_response_bucket_headers
 test_baton_t *tb = tc->testBaton;
 serf_bucket_t *bkt, *tmp, *hdr;
 
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 
 tmp = SERF_BUCKET_SIMPLE_STRING(
 "HTTP/1.1 405 Method Not Allowed" CRLF
@@ -342,8 +339,7 @@ static void test_response_bucket_chunked
 test_baton_t *tb = tc->testBaton;
 serf_bucket_t *bkt, *tmp, *hdrs;
 
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 
 tmp = SERF_BUCKET_SIMPLE_STRING(
 "HTTP/1.1 200 OK" CRLF
@@ -374,8 +370,7 @@ static void test_response_bucket_chunked
 static void test_bucket_header_set(CuTest *tc)
 {
 test_baton_t *tb = tc->testBaton;
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 serf_bucket_t *hdrs = serf_bucket_headers_create(alloc);
 
 CuAssertTrue(tc, hdrs != NULL);
@@ -417,8 +412,7 @@ store_header_in_table(void *baton, const
 static void test_bucket_header_do(CuTest *tc)
 {
 test_baton_t *tb = tc->testBaton;
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 serf_bucket_t *hdrs = serf_bucket_headers_create(alloc);
 struct kv {
 const char *key;
@@ -472,8 +466,7 @@ static void test_iovec_buckets(CuTest *t
 int i;
 int vecs_used;
 
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 
 /* Test 1: Read a single string in an iovec, store it in a iovec_bucket
and then read it back. */
@@ -658,8 +651,7 @@ static void test_iovec_buckets(CuTest *t
 static void test_header_buckets(CuTest *tc)
 {
 test_baton_t *tb = tc->testBaton;
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-  NULL);
+serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 const char *cur;
 
 serf_bucket_t *hdrs = serf_bucket_headers_create(alloc);
@@ -688,8 +680,7 @@ static void test_aggregate_buckets(CuTes
 apr_size_t len;
 const char *data;
 
-serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
-

buildbot failure in ASF Buildbot on serf-x64-macosx-apr2.0-dev

2015-11-03 Thread buildbot 
The Buildbot has detected a new failure on builder serf-x64-macosx-apr2.0-dev 
while building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/181

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712312
Blamelist: ivan

BUILD FAILED: failed Test

Sincerely,
 -The Buildbot

svn commit: r1712435 - in /serf/trunk: buckets/hpack_buckets.c buckets/prefix_buckets.c test/test_buckets.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Tue Nov  3 23:09:09 2015
New Revision: 1712435

URL: http://svn.apache.org/viewvc?rev=1712435=rev
Log:
Resolve two memory leaks found by ivan.

While looking at this code also shrink the amount of memory needed at
runtime while decoding http2 hpack data.

* buckets/hpack_buckets.c
  (serf_hpack_context_t): Remove unnecessary reference. This value is stored
in the bucket.
  (serf__bucket_hpack_create,
   serf__bucket_hpack_setx,
   serialize,
   serf_hpack_destroy_and_data): Update usage.

  (serf_hpack_decode_ctx_t): Remove variable.
  (serf__bucket_hpack_decode_create): Start by using a cheap small buffer.
  (hpack_decode_buffer_ensure): New helper function.
  (handle_read_entry_and_clear): Add argument. Update allocator usages.
  (hpack_process): Update caller. Ensure buffer when we know its size.
  (serf_hpack_decode_destroy): Relase the buffer when done.

* buckets/prefix_buckets.c
  (serf_prefix_destroy): Release stream when done.

* test/test_buckets.c
  (test_prefix_buckets): Recreate aggregate to handle changed behavior.

Modified:
serf/trunk/buckets/hpack_buckets.c
serf/trunk/buckets/prefix_buckets.c
serf/trunk/test/test_buckets.c

Modified: serf/trunk/buckets/hpack_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/hpack_buckets.c?rev=1712435=1712434=1712435=diff
==
--- serf/trunk/buckets/hpack_buckets.c (original)
+++ serf/trunk/buckets/hpack_buckets.c Tue Nov  3 23:09:09 2015
@@ -523,7 +523,6 @@ hpack_table_get(apr_uint64_t v,
 
 typedef struct serf_hpack_context_t
 {
-  serf_bucket_alloc_t *alloc;
   serf_hpack_table_t *tbl;
 
   serf_hpack_entry_t *first;
@@ -592,7 +591,6 @@ serf__bucket_hpack_create(serf_hpack_tab
 {
   serf_hpack_context_t *ctx = serf_bucket_mem_alloc(allocator, sizeof(*ctx));
 
-  ctx->alloc = allocator;
   ctx->tbl = hpack_table;
   ctx->first = ctx->last = NULL;
 
@@ -611,7 +609,7 @@ serf__bucket_hpack_setc(serf_bucket_t *h
 }
 
 void
-serf__bucket_hpack_setx(serf_bucket_t *hpack_bucket,
+serf__bucket_hpack_setx(serf_bucket_t *bucket,
 const char *key,
 apr_size_t key_size,
 int key_copy,
@@ -621,7 +619,7 @@ serf__bucket_hpack_setx(serf_bucket_t *h
 int dont_index,
 int never_index)
 {
-  serf_hpack_context_t *ctx = hpack_bucket->data;
+  serf_hpack_context_t *ctx = bucket->data;
   serf_hpack_entry_t *entry;
   apr_size_t i;
 
@@ -639,9 +637,9 @@ serf__bucket_hpack_setx(serf_bucket_t *h
   if (entry && value[0] == ':')
 {
   if (entry->free_val)
-serf_bucket_mem_free(ctx->alloc, (void*)entry->value);
+serf_bucket_mem_free(bucket->allocator, (void*)entry->value);
 
-  entry->value = serf_bstrmemdup(ctx->alloc, value, value_size);
+  entry->value = serf_bstrmemdup(bucket->allocator, value, value_size);
   entry->value_len = value_size;
   entry->free_val = TRUE;
   entry->dont_index = never_index ? 2 : (dont_index ? 1 : 0);
@@ -653,7 +651,7 @@ serf__bucket_hpack_setx(serf_bucket_t *h
   /* We probably want to allow duplicate *and* join behavior? */
 }
 
-  entry = serf_bucket_mem_calloc(ctx->alloc, sizeof(*entry));
+  entry = serf_bucket_mem_calloc(bucket->allocator, sizeof(*entry));
 
   if (ctx->tbl && ctx->tbl->lowercase_keys)
 {
@@ -664,7 +662,7 @@ serf__bucket_hpack_setx(serf_bucket_t *h
  encoding in HTTP/2.  A request or response containing uppercase
  header field names MUST be treated as malformed (Section 8.1.2.6). */
 
-  char *ckey = serf_bstrmemdup(ctx->alloc, key, key_size);
+  char *ckey = serf_bstrmemdup(bucket->allocator, key, key_size);
   for (i = 0; i < key_size; i++)
 {
   if (ckey[i] >= 'A' && key[i] <= 'Z')
@@ -680,14 +678,14 @@ serf__bucket_hpack_setx(serf_bucket_t *h
 }
   else
 {
-  entry->key = serf_bstrmemdup(ctx->alloc, key, key_size);
+  entry->key = serf_bstrmemdup(bucket->allocator, key, key_size);
   entry->free_key = TRUE;
 }
 
   entry->key_len = key_size;
   if (value_copy)
 {
-  entry->value = serf_bstrmemdup(ctx->alloc, value, value_size);
+  entry->value = serf_bstrmemdup(bucket->allocator, value, value_size);
   entry->free_val = TRUE;
 }
   else
@@ -783,7 +781,7 @@ serialize(serf_bucket_t *bucket)
  efficient HTTP2 / HPACK header format 
*/
   serf_hpack_context_t *ctx = bucket->data;
-  serf_bucket_alloc_t *alloc = ctx->alloc;
+  serf_bucket_alloc_t *alloc = bucket->allocator;
 
   serf_hpack_entry_t *entry;
   serf_hpack_entry_t *next;
@@ -897,7 +895,7 @@ serf_hpack_destroy_and_data(serf_bucket_
 {
   next = hi->next;
 
-  hpack_free_entry(hi, ctx->alloc);
+  hpack_free_entry(hi, bucket->allocator);
 }
 
   serf_default_destroy_and_data(bucket);
@@ -922,7 +920,6 @@ const serf_bucket_type_t serf_bucket_typ
 
 typedef

RE: svn commit: r1712270 - in /serf/trunk: buckets/allocator.c buckets/buckets.c test/test_util.c

2015-11-03 Thread Bert Huijben 
No this wasn’t needed.

 

I first intended to add the macro there, but then I found out that it only 
really applied to debugging.

 

Thanks for the review!

 

Bert

 

From: Greg Stein [mailto:gst...@gmail.com] 
Sent: dinsdag 3 november 2015 13:41
To: Bert Huijben 
Cc: dev@serf.apache.org
Subject: Re: svn commit: r1712270 - in /serf/trunk: buckets/allocator.c 
buckets/buckets.c test/test_util.c

 

On Tue, Nov 3, 2015 at 6:12 AM,  > wrote:

>...

+++ serf/trunk/buckets/allocator.c Tue Nov  3 12:12:00 2015
@@ -27,6 +27,8 @@
 #include "serf.h"
 #include "serf_bucket_util.h"

+#include "serf_private.h"

 

Is this really needed? The changes don't seem to require exposure to the 
private header.

 

>...

 

Cheers,

-g

buildbot success in ASF Buildbot on serf-x64-macosx-apr2.0-dev

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder 
serf-x64-macosx-apr2.0-dev while building ASF Buildbot. Full details are 
available at:
http://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/189

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712455
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

buildbot failure in ASF Buildbot on serf-x64-macosx-apr2.0-dev

2015-11-03 Thread buildbot 
The Buildbot has detected a new failure on builder serf-x64-macosx-apr2.0-dev 
while building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/188

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712453
Blamelist: rhuijben

BUILD FAILED: failed Test

Sincerely,
 -The Buildbot

svn commit: r1712453 - in /serf/trunk: buckets/aggregate_buckets.c test/MockHTTPinC/MockHTTP.c test/MockHTTPinC/MockHTTP_server.c test/mock_buckets.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Wed Nov  4 01:19:55 2015
New Revision: 1712453

URL: http://svn.apache.org/viewvc?rev=1712453=rev
Log:
Fix several cases where we checked for APR_EAGAIN instead of
APR_STATUS_IS_EAGAIN, where this last one doesn't work for socket errors
on Windows.

This caused the MockHTTP_server to never generate WANT_READ errors on
Windows, while it did produce them on all our other buildbots.

* buckets/aggregate_buckets.c
  (serf_aggregate_peek): Use proper check for EAGAIN.

* test/MockHTTPinC/MockHTTP.c
  (mhRunServerLoopCompleteRequests): Use proper check for EAGAIN.

* test/MockHTTPinC/MockHTTP_server.c
  (processServer,
   bio_apr_socket_read): Use proper check for EAGAIN.

* test/mock_buckets.c
  (serf_bucket_mock_more_data_arrived): Fix check.

Modified:
serf/trunk/buckets/aggregate_buckets.c
serf/trunk/test/MockHTTPinC/MockHTTP.c
serf/trunk/test/MockHTTPinC/MockHTTP_server.c
serf/trunk/test/mock_buckets.c

Modified: serf/trunk/buckets/aggregate_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/aggregate_buckets.c?rev=1712453=1712452=1712453=diff
==
--- serf/trunk/buckets/aggregate_buckets.c (original)
+++ serf/trunk/buckets/aggregate_buckets.c Wed Nov  4 01:19:55 2015
@@ -440,7 +440,7 @@ static apr_status_t serf_aggregate_peek(
 *len = 0;
 if (ctx->hold_open) {
 status = ctx->hold_open(ctx->hold_open_baton, bucket);
-if (status == APR_EAGAIN)
+if (APR_STATUS_IS_EAGAIN(status))
 status = APR_SUCCESS;
 return status;
 }
@@ -459,7 +459,7 @@ static apr_status_t serf_aggregate_peek(
 } else {
 if (ctx->hold_open) {
 status = ctx->hold_open(ctx->hold_open_baton, bucket);
-if (status == APR_EAGAIN)
+if (APR_STATUS_IS_EAGAIN(status))
 status = APR_SUCCESS;
 return status;
 }

Modified: serf/trunk/test/MockHTTPinC/MockHTTP.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/MockHTTPinC/MockHTTP.c?rev=1712453=1712452=1712453=diff
==
--- serf/trunk/test/MockHTTPinC/MockHTTP.c (original)
+++ serf/trunk/test/MockHTTPinC/MockHTTP.c Wed Nov  4 01:19:55 2015
@@ -180,7 +180,7 @@ mhError_t mhRunServerLoopCompleteRequest
 break;
 };
 
-if (status == APR_EAGAIN)
+if (APR_STATUS_IS_EAGAIN(status))
 return MOCKHTTP_TIMEOUT;
 
 return status;

Modified: serf/trunk/test/MockHTTPinC/MockHTTP_server.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/MockHTTPinC/MockHTTP_server.c?rev=1712453=1712452=1712453=diff
==
--- serf/trunk/test/MockHTTPinC/MockHTTP_server.c (original)
+++ serf/trunk/test/MockHTTPinC/MockHTTP_server.c Wed Nov  4 01:19:55 2015
@@ -1403,7 +1403,7 @@ static apr_status_t processServer(mhServ
 cctx->req = NULL;
 
 return APR_SUCCESS;
-} else if (status == APR_SUCCESS || status == APR_EAGAIN) {
+} else if (status == APR_SUCCESS || APR_STATUS_IS_EAGAIN(status)) {
 ctx->reqState = PartialReqReceived;
 }
 
@@ -2314,11 +2314,11 @@ static int bio_apr_socket_read(BIO *bio,
 status = apr_socket_recv(cctx->skt, in, );
 ssl_ctx->bio_status = status;
 
-if (len || status != APR_EAGAIN)
+if (len || APR_STATUS_IS_EAGAIN(status))
 _mhLog(MH_VERBOSE, cctx->skt, "Read %d bytes from ssl socket with "
"status %d.\n", len, status);
 
-if (status == APR_EAGAIN) {
+if (APR_STATUS_IS_EAGAIN(status)) {
 BIO_set_retry_read(bio);
 if (len == 0)
 return -1;

Modified: serf/trunk/test/mock_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/mock_buckets.c?rev=1712453=1712452=1712453=diff
==
--- serf/trunk/test/mock_buckets.c (original)
+++ serf/trunk/test/mock_buckets.c Wed Nov  4 01:19:55 2015
@@ -181,7 +181,7 @@ apr_status_t serf_bucket_mock_more_data_
 return status;
 
 action = >actions[ctx->current_action];
-if (ctx->remaining_data == 0 && action->status == APR_EAGAIN) {
+if (ctx->remaining_data == 0 && APR_STATUS_IS_EAGAIN(action->status)) {
 ctx->remaining_times--;
 action->times--;
 }

svn commit: r1712456 - /serf/trunk/buckets/aggregate_buckets.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Wed Nov  4 02:13:20 2015
New Revision: 1712456

URL: http://svn.apache.org/viewvc?rev=1712456=rev
Log:
* buckets/aggregate_buckets.c
  (read_aggregate): Add 'magic' refill support via the hold_open callback.
   when reading an iovec. In the http2 usage this might just continue with
   the next data frame.

Modified:
serf/trunk/buckets/aggregate_buckets.c

Modified: serf/trunk/buckets/aggregate_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/aggregate_buckets.c?rev=1712456=1712455=1712456=diff
==
--- serf/trunk/buckets/aggregate_buckets.c (original)
+++ serf/trunk/buckets/aggregate_buckets.c Wed Nov  4 02:13:20 2015
@@ -294,7 +294,10 @@ static apr_status_t read_aggregate(serf_
 /* If we have no more in our list, return EOF. */
 if (!ctx->list) {
 if (ctx->hold_open) {
-return ctx->hold_open(ctx->hold_open_baton, bucket);
+status = ctx->hold_open(ctx->hold_open_baton, bucket);
+if (status || !ctx->list)
+return status;
+/* Wow, we 'magically' refilled! */
 }
 else {
 return APR_EOF;

buildbot success in ASF Buildbot on serf-x64-macosx-apr1.5

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder serf-x64-macosx-apr1.5 
while building ASF Buildbot. Full details are available at:
http://ci.apache.org/builders/serf-x64-macosx-apr1.5/builds/190

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712439
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

svn commit: r1712450 - /serf/trunk/test/MockHTTPinC/MockHTTP_server.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Wed Nov  4 00:55:46 2015
New Revision: 1712450

URL: http://svn.apache.org/viewvc?rev=1712450=rev
Log:
Following up on r1712447, remove write buffer but add a bit more logging
to the read buffer test.

* test/MockHTTPinC/MockHTTP_server.c
  (sslCtx_t): Add retry_len to exactly match arguments when needed.
  (initSSLCtx): Apply a few more mode flags.
  (sslSocketRead): Tweak a bit for debugging.

Modified:
serf/trunk/test/MockHTTPinC/MockHTTP_server.c

Modified: serf/trunk/test/MockHTTPinC/MockHTTP_server.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/MockHTTPinC/MockHTTP_server.c?rev=1712450=1712449=1712450=diff
==
--- serf/trunk/test/MockHTTPinC/MockHTTP_server.c (original)
+++ serf/trunk/test/MockHTTPinC/MockHTTP_server.c Wed Nov  4 00:55:46 2015
@@ -2230,8 +2230,8 @@ struct sslCtx_t {
 SSL* ssl;
 BIO *bio;
 
+apr_size_t retry_len;
 char read_buffer[8192];
-char write_buffer[8192];
 };
 
 static int init_done = 0;
@@ -2699,7 +2699,9 @@ static apr_status_t initSSLCtx(_mhClient
 }
 #endif
 
-SSL_CTX_set_mode(ssl_ctx->ctx, SSL_MODE_ENABLE_PARTIAL_WRITE);
+SSL_CTX_set_mode(ssl_ctx->ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER
+   | SSL_MODE_ENABLE_PARTIAL_WRITE
+   | SSL_MODE_AUTO_RETRY);
 
 ssl_ctx->bio = BIO_new(_apr_socket_method);
 ssl_ctx->bio->ptr = cctx;
@@ -2720,15 +2722,11 @@ sslSocketWrite(_mhClientCtx_t *cctx, con
 sslCtx_t *ssl_ctx = cctx->ssl_ctx;
 int result, ssl_err;
 
-if (*len > sizeof(ssl_ctx->write_buffer))
-  *len = sizeof(ssl_ctx->write_buffer);
-memcpy(ssl_ctx->write_buffer, data, *len);
-
 if (*len == 0)
 return APR_SUCCESS;
 
 ssl_ctx->bio_status = APR_SUCCESS;
-result = SSL_write(ssl_ctx->ssl, ssl_ctx->write_buffer, *len);
+result = SSL_write(ssl_ctx->ssl, data, *len);
 if (result > 0) {
 *len = result;
 return APR_SUCCESS;
@@ -2784,12 +2782,15 @@ sslSocketRead(apr_socket_t *skt, void *b
 
 if (*len > sizeof(ssl_ctx->read_buffer))
   *len = sizeof(ssl_ctx->read_buffer);
+if (ssl_ctx->retry_len)
+  *len = ssl_ctx->retry_len;
 
 ssl_ctx->bio_status = APR_SUCCESS;
 result = SSL_read(ssl_ctx->ssl, ssl_ctx->read_buffer, *len);
 if (result > 0) {
 *len = result;
 memcpy(data, ssl_ctx->read_buffer, *len);
+ssl_ctx->retry_len = 0;
 return APR_SUCCESS;
 } else {
 int ssl_err;
@@ -2805,6 +2806,9 @@ sslSocketRead(apr_socket_t *skt, void *b
 return ssl_ctx->bio_status;
 case SSL_ERROR_WANT_READ:
 case SSL_ERROR_WANT_WRITE:
+ssl_ctx->retry_len = *len;
+fprintf(stderr, "WANT %s\n", (ssl_err== SSL_ERROR_WANT_READ)
+   ? "read" : "write");
 *len = 0;
 return APR_EAGAIN;
 case SSL_ERROR_SSL:

buildbot success in ASF Buildbot on serf-x64-macosx-default-openssl

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder 
serf-x64-macosx-default-openssl while building ASF Buildbot. Full details are 
available at:
http://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/195

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712450
Blamelist: rhuijben

Build succeeded!

Sincerely,
 -The Buildbot

svn commit: r1712455 - /serf/trunk/test/MockHTTPinC/MockHTTP_server.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Wed Nov  4 01:56:18 2015
New Revision: 1712455

URL: http://svn.apache.org/viewvc?rev=1712455=rev
Log:
* test/MockHTTPinC/MockHTTP_server.c
  (bio_apr_socket_write): Remove retry_len hack.
  (bio_apr_socket_read,
   bio_apr_socket_write): Make code more similar. Explicitly return -1
 on errors to make the wrapper return the cached apr status.
  (sslSocketRead): Remove retry_len caching and printf.

Modified:
serf/trunk/test/MockHTTPinC/MockHTTP_server.c

Modified: serf/trunk/test/MockHTTPinC/MockHTTP_server.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/MockHTTPinC/MockHTTP_server.c?rev=1712455=1712454=1712455=diff
==
--- serf/trunk/test/MockHTTPinC/MockHTTP_server.c (original)
+++ serf/trunk/test/MockHTTPinC/MockHTTP_server.c Wed Nov  4 01:56:18 2015
@@ -2230,7 +2230,6 @@ struct sslCtx_t {
 SSL* ssl;
 BIO *bio;
 
-apr_size_t retry_len;
 char read_buffer[8192];
 };
 
@@ -2314,20 +2313,18 @@ static int bio_apr_socket_read(BIO *bio,
 status = apr_socket_recv(cctx->skt, in, );
 ssl_ctx->bio_status = status;
 
-if (len || APR_STATUS_IS_EAGAIN(status))
+if (len > 0)
 _mhLog(MH_VERBOSE, cctx->skt, "Read %d bytes from ssl socket with "
"status %d.\n", len, status);
 
 if (APR_STATUS_IS_EAGAIN(status)) {
 BIO_set_retry_read(bio);
-if (len == 0)
-return -1;
 }
 
 if (READ_ERROR(status))
 return -1;
 
-return len;
+return len ? len : -1;
 }
 
 /**
@@ -2345,16 +2342,18 @@ static int bio_apr_socket_write(BIO *bio
 status = apr_socket_send(cctx->skt, in, );
 ssl_ctx->bio_status = status;
 
-if (len || !APR_STATUS_IS_EAGAIN(status))
+if (len > 0)
 _mhLog(MH_VERBOSE, cctx->skt, "Wrote %d of %d bytes to ssl socket with 
"
"status %d.\n", len, inlen, status);
-else if (APR_STATUS_IS_EAGAIN(status))
-  BIO_set_retry_write(bio);
+
+if (APR_STATUS_IS_EAGAIN(status)) {
+BIO_set_retry_write(bio);
+}
 
 if (READ_ERROR(status))
 return -1;
 
-return len;
+return len ? len : -1;
 }
 
 
@@ -2782,15 +2781,12 @@ sslSocketRead(apr_socket_t *skt, void *b
 
 if (*len > sizeof(ssl_ctx->read_buffer))
   *len = sizeof(ssl_ctx->read_buffer);
-if (ssl_ctx->retry_len)
-  *len = ssl_ctx->retry_len;
 
 ssl_ctx->bio_status = APR_SUCCESS;
 result = SSL_read(ssl_ctx->ssl, ssl_ctx->read_buffer, *len);
 if (result > 0) {
 *len = result;
 memcpy(data, ssl_ctx->read_buffer, *len);
-ssl_ctx->retry_len = 0;
 return APR_SUCCESS;
 } else {
 int ssl_err;
@@ -2806,9 +2802,6 @@ sslSocketRead(apr_socket_t *skt, void *b
 return ssl_ctx->bio_status;
 case SSL_ERROR_WANT_READ:
 case SSL_ERROR_WANT_WRITE:
-ssl_ctx->retry_len = *len;
-fprintf(stderr, "WANT %s\n", (ssl_err== SSL_ERROR_WANT_READ)
-   ? "read" : "write");
 *len = 0;
 return APR_EAGAIN;
 case SSL_ERROR_SSL:

svn commit: r1712458 - /serf/trunk/test/test_buckets.c

2015-11-03 Thread rhuijben 
Author: rhuijben
Date: Wed Nov  4 02:30:20 2015
New Revision: 1712458

URL: http://svn.apache.org/viewvc?rev=1712458=rev
Log:
Following up on r1712456, add test for refilling during hold-open.

* test/test_buckets.c
  (append_magic): New function.
  (test_aggregate_buckets): Extend test.

Modified:
serf/trunk/test/test_buckets.c

Modified: serf/trunk/test/test_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1712458=1712457=1712458=diff
==
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Wed Nov  4 02:30:20 2015
@@ -670,6 +670,24 @@ static void test_header_buckets(CuTest *
 serf_bucket_destroy(hdrs);
 }
 
+static apr_status_t append_magic(void *baton,
+ serf_bucket_t *bucket)
+{
+  serf_bucket_t *bkt;
+  int *append = baton;
+
+  if (*append)
+{
+  (*append)--;
+  bkt = SERF_BUCKET_SIMPLE_STRING("magic", bucket->allocator);
+  serf_bucket_aggregate_append(bucket, bkt);
+  return APR_SUCCESS;
+}
+
+  return APR_EOF;
+}
+
+
 static void test_aggregate_buckets(CuTest *tc)
 {
 test_baton_t *tb = tc->testBaton;
@@ -679,6 +697,7 @@ static void test_aggregate_buckets(CuTes
 int vecs_used;
 apr_size_t len;
 const char *data;
+int append = 3;
 
 serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
 const char *BODY = "12345678901234567890"\
@@ -784,6 +803,23 @@ static void test_aggregate_buckets(CuTes
 /* While it doesn't affect the inner bucket */
 read_and_check_bucket(tc, bkt, BODY);
 serf_bucket_destroy(bkt);
+
+aggbkt = serf_bucket_aggregate_create(alloc);
+bkt = SERF_BUCKET_SIMPLE_STRING_LEN(BODY, 15, alloc);
+serf_bucket_aggregate_append(aggbkt, bkt);
+
+serf_bucket_aggregate_hold_open(aggbkt, append_magic, );
+
+CuAssertIntEquals(tc, APR_EOF,
+  serf_bucket_read_iovec(aggbkt, SERF_READ_ALL_AVAIL,
+ 32, tgt_vecs, _used));
+CuAssertIntEquals(tc, 4, vecs_used);
+CuAssertIntEquals(tc, 15, tgt_vecs[0].iov_len);
+CuAssertIntEquals(tc, 5, tgt_vecs[1].iov_len);
+CuAssertIntEquals(tc, 5, tgt_vecs[2].iov_len);
+CuAssertIntEquals(tc, 5, tgt_vecs[3].iov_len);
+
+serf_bucket_destroy(aggbkt);
 }
 
 static void test_aggregate_bucket_readline(CuTest *tc)

RE: serf_bucket_copy_create() wrapped bucket ownership

2015-11-03 Thread Bert Huijben 


> -Original Message-
> From: Ivan Zhakov [mailto:i...@visualsvn.com]
> Sent: dinsdag 3 november 2015 15:01
> To: dev@serf.apache.org
> Subject: serf_bucket_copy_create() wrapped bucket ownership
> 
> Should serf_bucket_destroy() for COPY_BUCKET destroy wrapped bucket or
> not? Currently it doesn't and this is inconsistent with other buckets
> behavior.

I think it should.

Only in very specific cases a bucket shouldn't destroy its inner stream.

Buckets that explicitly read some prefix of another stream that is not yet 
closed (or more in particular the http2 unframe stream) is one of such 
exceptions, but I don't see why the copy stream should be one of them.

Bert

> 
> --
> Ivan Zhakov

buildbot success in ASF Buildbot on serf-x64-macosx-apr2.0-dev

2015-11-03 Thread buildbot 
The Buildbot has detected a restored build on builder 
serf-x64-macosx-apr2.0-dev while building ASF Buildbot. Full details are 
available at:
http://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/182

Buildbot URL: http://ci.apache.org/

Buildslave for this Build: svn-x64-macosx-dgvrs

Build Reason: The AnyBranchScheduler scheduler named 'on-serf-commit' triggered 
this build
Build Source Stamp: [branch serf/trunk] 1712318
Blamelist: ivan

Build succeeded!

Sincerely,
 -The Buildbot

svn commit: r1712320 - in /serf/trunk: buckets/copy_buckets.c test/test_buckets.c

2015-11-03 Thread ivan 
Author: ivan
Date: Tue Nov  3 16:22:34 2015
New Revision: 1712320

URL: http://svn.apache.org/viewvc?rev=1712320=rev
Log:
Destroy wrapped bucket in copy bucket.

Discussion:
http://mail-archives.apache.org/mod_mbox/serf-dev/201511.mbox/%3ccabw-3ycbvkfjze0ttergbguej_fz7dg8vf_bmrab2qra6nb...@mail.gmail.com%3E

* buckets/copy_buckets.c
  (serf_copy_destroy): Destroy CTX->WRAPPED bucket.

* test/test_buckets.c
  (test_copy_bucket): Destroy tested copy bucket and do not reuse wrapped
   aggregate bucket since it will be destroyed.

Modified:
serf/trunk/buckets/copy_buckets.c
serf/trunk/test/test_buckets.c

Modified: serf/trunk/buckets/copy_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/copy_buckets.c?rev=1712320=1712319=1712320=diff
==
--- serf/trunk/buckets/copy_buckets.c (original)
+++ serf/trunk/buckets/copy_buckets.c Tue Nov  3 16:22:34 2015
@@ -313,6 +313,7 @@ static void serf_copy_destroy(serf_bucke
 if (ctx->hold_buf)
 serf_bucket_mem_free(bucket->allocator, ctx->hold_buf);
 
+serf_bucket_destroy(ctx->wrapped);
 serf_default_destroy_and_data(bucket);
 }
 

Modified: serf/trunk/test/test_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1712320=1712319=1712320=diff
==
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Tue Nov  3 16:22:34 2015
@@ -1125,8 +1125,10 @@ static void test_copy_bucket(CuTest *tc)
 CuAssertIntEquals(tc, APR_EOF, status);
 CuAssertIntEquals(tc, strlen(BODY), len);
 
+serf_bucket_destroy(copybkt);
 
-/* Just reuse the existing aggbkt. Fill again */
+/* Fill again aggregate bucket again. */
+aggbkt = serf_bucket_aggregate_create(alloc);
 copybkt = serf_bucket_copy_create(aggbkt, 35, alloc);
 bkt = SERF_BUCKET_SIMPLE_STRING_LEN(BODY, 20, alloc);
 serf_bucket_aggregate_append(aggbkt, bkt);
@@ -1148,7 +1150,10 @@ static void test_copy_bucket(CuTest *tc)
 CuAssertIntEquals(tc, APR_EOF, discard_data(copybkt, ));
 CuAssertIntEquals(tc, strlen(BODY) - 35, len);
 
-/* Just reuse the existing aggbkt. Fill again */
+serf_bucket_destroy(copybkt);
+
+/* Fill again aggregate bucket again. */
+aggbkt = serf_bucket_aggregate_create(alloc);
 copybkt = serf_bucket_copy_create(aggbkt, 45, alloc);
 bkt = SERF_BUCKET_SIMPLE_STRING_LEN(BODY, 20, alloc);
 serf_bucket_aggregate_append(aggbkt, bkt);

Re: serf_bucket_copy_create() wrapped bucket ownership

2015-11-03 Thread Ivan Zhakov 
On 3 November 2015 at 17:56, Bert Huijben  wrote:
>> -Original Message-
>> From: Ivan Zhakov [mailto:i...@visualsvn.com]
>> Sent: dinsdag 3 november 2015 15:01
>> To: dev@serf.apache.org
>> Subject: serf_bucket_copy_create() wrapped bucket ownership
>>
>> Should serf_bucket_destroy() for COPY_BUCKET destroy wrapped bucket or
>> not? Currently it doesn't and this is inconsistent with other buckets
>> behavior.
>
> I think it should.
>
> Only in very specific cases a bucket shouldn't destroy its inner stream.
>
> Buckets that explicitly read some prefix of another stream that is not yet
> closed (or more in particular the http2 unframe stream) is one of such
> exceptions, but I don't see why the copy stream should be one of them.
>
Fixed in r1712320.

The same problem exists in prefix buckets: should I fix them also?

-- 
Ivan Zhakov