Author: rhuijben Date: Sun Nov 1 14:27:51 2015 New Revision: 1711772 URL: http://svn.apache.org/viewvc?rev=1711772&view=rev Log: Cleanup some constructor arguments of http2 decompression buckets.
Destroying the stream with an unframe bucket doesn't make sense and not destroying the frame with the padding bucket is so uncommon that users should just use a barrier bucket if they need that behavior. * buckets/http2_frame_buckets.c (http2_unframe_context_t): Use typedef for callback. Remove unneeded bool. (serf__bucket_http2_unframe_create): Remove unneeded argument. (serf_http2_unframe_read, serf_http2_unframe_read_iovec): Update caller. (serf_http2_unframe_destroy): Remove now unused function. (serf_bucket_type__http2_unframe): Use serf_default_destroy_and_data directly (http2_unpad_context_t): Remove unneeded value. (serf__bucket_http2_unpad_create): Update usage. Remove argument. (serf_http2_unpad_destroy): Remove if. * protocols/http2_buckets.h (serf_bucket_end_of_frame_t): New typedef. (serf__bucket_http2_unframe_create): Remove argument. Tweak docs. (serf__bucket_http2_unframe_set_eof): Update argument type. (serf__bucket_http2_unpad_create): Remove argument. * protocols/http2_protocol.c (http2_process): Update caller. * test/test_buckets.c (test_http2_unframe_buckets, test_http2_unpad_buckets, test_http2_frame_bucket_basic): Update caller. Modified: serf/trunk/buckets/http2_frame_buckets.c serf/trunk/protocols/http2_buckets.h serf/trunk/protocols/http2_protocol.c serf/trunk/test/test_buckets.c Modified: serf/trunk/buckets/http2_frame_buckets.c URL: http://svn.apache.org/viewvc/serf/trunk/buckets/http2_frame_buckets.c?rev=1711772&r1=1711771&r2=1711772&view=diff ============================================================================== --- serf/trunk/buckets/http2_frame_buckets.c (original) +++ serf/trunk/buckets/http2_frame_buckets.c Sun Nov 1 14:27:51 2015 @@ -38,9 +38,8 @@ typedef struct http2_unframe_context_t apr_size_t prefix_remaining; - apr_status_t (*eof_callback)(void *baton, - serf_bucket_t *bucket); - void *eof_callback_baton; + serf_bucket_end_of_frame_t end_of_frame; + void *end_of_frame_baton; /* These fields are only set after prefix_remaining is 0 */ apr_size_t payload_remaining; /* 0 <= payload_length < 2^24 */ @@ -49,12 +48,10 @@ typedef struct http2_unframe_context_t unsigned char flags; unsigned char buffer[FRAME_PREFIX_SIZE]; - char destroy_stream; } http2_unframe_context_t; serf_bucket_t * serf__bucket_http2_unframe_create(serf_bucket_t *stream, - int destroy_stream, apr_size_t max_payload_size, serf_bucket_alloc_t *allocator) { @@ -64,24 +61,20 @@ serf__bucket_http2_unframe_create(serf_b ctx->stream = stream; ctx->max_payload_size = max_payload_size; ctx->prefix_remaining = sizeof(ctx->buffer); - ctx->eof_callback = NULL; - - ctx->destroy_stream = (destroy_stream != 0); + ctx->end_of_frame = NULL; return serf_bucket_create(&serf_bucket_type__http2_unframe, allocator, ctx); } void serf__bucket_http2_unframe_set_eof(serf_bucket_t *bucket, - apr_status_t (*eof_callback)( - void *baton, - serf_bucket_t *bucket), - void *eof_callback_baton) + serf_bucket_end_of_frame_t end_of_frame, + void *end_of_frame_baton) { http2_unframe_context_t *ctx = bucket->data; - ctx->eof_callback = eof_callback; - ctx->eof_callback_baton = eof_callback_baton; + ctx->end_of_frame = end_of_frame; + ctx->end_of_frame_baton = end_of_frame_baton; } apr_status_t @@ -163,12 +156,12 @@ serf__bucket_http2_unframe_read_info(ser /* If we hava a zero-length frame we have to call the eof callback now, as the read operations will just shortcut to APR_EOF */ - if (ctx->payload_remaining == 0 && ctx->eof_callback) + if (ctx->payload_remaining == 0 && ctx->end_of_frame) { apr_status_t cb_status; - cb_status = ctx->eof_callback(ctx->eof_callback_baton, - bucket); + cb_status = (*ctx->end_of_frame)(ctx->end_of_frame_baton, + bucket); if (SERF_BUCKET_READ_ERROR(cb_status)) status = cb_status; @@ -220,9 +213,9 @@ serf_http2_unframe_read(serf_bucket_t *b if (ctx->payload_remaining == 0) { - if (ctx->eof_callback) - status = ctx->eof_callback(ctx->eof_callback_baton, - bucket); + if (ctx->end_of_frame) + status = (*ctx->end_of_frame)(ctx->end_of_frame_baton, + bucket); if (!SERF_BUCKET_READ_ERROR(status)) status = APR_EOF; @@ -275,9 +268,9 @@ serf_http2_unframe_read_iovec(serf_bucke if (ctx->payload_remaining == 0) { - if (ctx->eof_callback) - status = ctx->eof_callback(ctx->eof_callback_baton, - bucket); + if (ctx->end_of_frame) + status = (*ctx->end_of_frame)(ctx->end_of_frame_baton, + bucket); if (!SERF_BUCKET_READ_ERROR(status)) status = APR_EOF; @@ -315,17 +308,6 @@ serf_http2_unframe_peek(serf_bucket_t *b return status; } -static void -serf_http2_unframe_destroy(serf_bucket_t *bucket) -{ - http2_unframe_context_t *ctx = bucket->data; - - if (ctx->destroy_stream) - serf_bucket_destroy(ctx->stream); - - serf_default_destroy_and_data(bucket); -} - static apr_uint64_t serf_http2_unframe_get_remaining(serf_bucket_t *bucket) { @@ -351,7 +333,7 @@ const serf_bucket_type_t serf_bucket_typ serf_default_read_for_sendfile, serf_buckets_are_v2, serf_http2_unframe_peek, - serf_http2_unframe_destroy, + serf_default_destroy_and_data, serf_default_read_bucket, serf_http2_unframe_get_remaining, serf_default_ignore_config @@ -363,13 +345,11 @@ typedef struct http2_unpad_context_t apr_size_t payload_remaining; apr_size_t pad_remaining; apr_size_t pad_length; - int padsize_read; - int destroy_stream; + char padsize_read; } http2_unpad_context_t; serf_bucket_t * serf__bucket_http2_unpad_create(serf_bucket_t *stream, - int destroy_stream, serf_bucket_alloc_t *allocator) { http2_unpad_context_t *ctx; @@ -377,7 +357,6 @@ serf__bucket_http2_unpad_create(serf_buc ctx = serf_bucket_mem_alloc(allocator, sizeof(*ctx)); ctx->stream = stream; ctx->padsize_read = FALSE; - ctx->destroy_stream = destroy_stream; return serf_bucket_create(&serf_bucket_type__http2_unpad, allocator, ctx); } @@ -589,8 +568,7 @@ serf_http2_unpad_destroy(serf_bucket_t * { http2_unpad_context_t *ctx = bucket->data; - if (ctx->destroy_stream) - serf_bucket_destroy(ctx->stream); + serf_bucket_destroy(ctx->stream); serf_default_destroy_and_data(bucket); } Modified: serf/trunk/protocols/http2_buckets.h URL: http://svn.apache.org/viewvc/serf/trunk/protocols/http2_buckets.h?rev=1711772&r1=1711771&r2=1711772&view=diff ============================================================================== --- serf/trunk/protocols/http2_buckets.h (original) +++ serf/trunk/protocols/http2_buckets.h Sun Nov 1 14:27:51 2015 @@ -38,9 +38,13 @@ extern "C" { extern const serf_bucket_type_t serf_bucket_type__http2_unframe; #define SERF__BUCKET_IS_HTTP2_UNFRAME(b) SERF_BUCKET_CHECK((b), _http2_unframe) -/* Creates a bucket that reads a single http2 frame from stream. If - DESTROY_STREAM is true STREAM will be destroyed with the bucket, otherwise - it won't. +typedef apr_status_t (*serf_bucket_end_of_frame_t)( + void *baton, + serf_bucket_t *unframe_bucket); + +/* Creates a bucket that reads a single http2 frame from stream. Note that + unlike many other buckets destroying the unframe bucket doesn't destroy the + underlying stream. The frame header information can be obtained by calling serf__bucket_http2_unframe_read_info(). @@ -50,7 +54,6 @@ extern const serf_bucket_type_t serf_buc */ serf_bucket_t * serf__bucket_http2_unframe_create(serf_bucket_t *stream, - int destroy_stream, apr_size_t max_payload_size, serf_bucket_alloc_t *allocator); @@ -58,10 +61,8 @@ serf__bucket_http2_unframe_create(serf_b the whole frame has been read from the contained stream */ void serf__bucket_http2_unframe_set_eof(serf_bucket_t *bucket, - apr_status_t (*eof_callback)( - void *baton, - serf_bucket_t *bucket), - void *eof_callback_baton); + serf_bucket_end_of_frame_t end_of_frame, + void *end_of_frame_baton); /* Obtains the frame header state, reading from the bucket if necessary. @@ -84,7 +85,6 @@ extern const serf_bucket_type_t serf_buc serf_bucket_t * serf__bucket_http2_unpad_create(serf_bucket_t *stream, - int destroy_stream, serf_bucket_alloc_t *allocator); /* ==================================================================== */ Modified: serf/trunk/protocols/http2_protocol.c URL: http://svn.apache.org/viewvc/serf/trunk/protocols/http2_protocol.c?rev=1711772&r1=1711771&r2=1711772&view=diff ============================================================================== --- serf/trunk/protocols/http2_protocol.c (original) +++ serf/trunk/protocols/http2_protocol.c Sun Nov 1 14:27:51 2015 @@ -814,7 +814,7 @@ http2_process(serf_http2_protocol_t *h2) SERF_H2_assert(!h2->in_frame); body = serf__bucket_http2_unframe_create( - h2->conn->stream, FALSE, + h2->conn->stream, h2->rl_max_framesize, h2->allocator); @@ -893,6 +893,7 @@ http2_process(serf_http2_protocol_t *h2) if (frametype == HTTP2_FRAME_TYPE_DATA) { + /* Windowing is applied above padding! */ remaining = (apr_size_t)serf_bucket_get_remaining(body); if (h2->rl_window < remaining) @@ -921,10 +922,7 @@ http2_process(serf_http2_protocol_t *h2) /* DATA, HEADERS and PUSH_PROMISE can have padding */ if (frameflags & HTTP2_FLAG_PADDED) - { - body = serf__bucket_http2_unpad_create(body, TRUE, - h2->allocator); - } + body = serf__bucket_http2_unpad_create(body, h2->allocator); /* An HEADERS frame can have an included priority 'frame' */ if (frametype == HTTP2_FRAME_TYPE_HEADERS Modified: serf/trunk/test/test_buckets.c URL: http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1711772&r1=1711771&r2=1711772&view=diff ============================================================================== --- serf/trunk/test/test_buckets.c (original) +++ serf/trunk/test/test_buckets.c Sun Nov 1 14:27:51 2015 @@ -1926,8 +1926,7 @@ static void test_http2_unframe_buckets(C raw = serf_bucket_simple_create(raw_frame1, sizeof(raw_frame1), NULL, NULL, alloc); - unframe = serf__bucket_http2_unframe_create(raw, TRUE, SERF_READ_ALL_AVAIL, - alloc); + unframe = serf__bucket_http2_unframe_create(raw, SERF_READ_ALL_AVAIL, alloc); CuAssertTrue(tc, SERF__BUCKET_IS_HTTP2_UNFRAME(unframe)); @@ -1955,8 +1954,7 @@ static void test_http2_unframe_buckets(C raw = serf_bucket_simple_create(raw_frame2, sizeof(raw_frame2), NULL, NULL, alloc); - unframe = serf__bucket_http2_unframe_create(raw, TRUE, SERF_READ_ALL_AVAIL, - alloc); + unframe = serf__bucket_http2_unframe_create(raw, SERF_READ_ALL_AVAIL, alloc); status = read_all(unframe, result2, sizeof(result2), &read_len); CuAssertIntEquals(tc, APR_EOF, status); @@ -1983,7 +1981,7 @@ static void test_http2_unframe_buckets(C raw = serf_bucket_simple_create(raw_frame2, sizeof(raw_frame2), NULL, NULL, alloc); - unframe = serf__bucket_http2_unframe_create(raw, TRUE, 5, alloc); + unframe = serf__bucket_http2_unframe_create(raw, 5, alloc); status = read_all(unframe, result2, sizeof(result2), &read_len); CuAssertIntEquals(tc, SERF_ERROR_HTTP2_FRAME_SIZE_ERROR, status); @@ -2019,8 +2017,7 @@ static void test_http2_unpad_buckets(CuT raw = serf_bucket_simple_create(raw_frame, sizeof(raw_frame)-1, NULL, NULL, alloc); - unframe = serf__bucket_http2_unframe_create(raw, FALSE, SERF_READ_ALL_AVAIL, - alloc); + unframe = serf__bucket_http2_unframe_create(raw, SERF_READ_ALL_AVAIL, alloc); { apr_int32_t streamid; @@ -2035,7 +2032,7 @@ static void test_http2_unpad_buckets(CuT CuAssertIntEquals(tc, 8, flags); } - unpad = serf__bucket_http2_unpad_create(unframe, TRUE, alloc); + unpad = serf__bucket_http2_unpad_create(unframe, alloc); CuAssertTrue(tc, SERF__BUCKET_IS_HTTP2_UNPAD(unpad)); @@ -2046,11 +2043,11 @@ static void test_http2_unpad_buckets(CuT read_and_check_bucket(tc, raw, "Extra"); raw = serf_bucket_simple_create("\0a", 2, NULL, NULL, alloc); - unpad = serf__bucket_http2_unpad_create(raw, TRUE, alloc); + unpad = serf__bucket_http2_unpad_create(raw, alloc); read_and_check_bucket(tc, unpad, "a"); raw = serf_bucket_simple_create("\5a", 2, NULL, NULL, alloc); - unpad = serf__bucket_http2_unpad_create(raw, TRUE, alloc); + unpad = serf__bucket_http2_unpad_create(raw, alloc); { const char *data; @@ -2314,7 +2311,7 @@ static void test_http2_frame_bucket_basi frame_in = serf__bucket_http2_frame_create(body_in, 99, 7, &exp_streamid, NULL, NULL, 16384, NULL, NULL, alloc); - frame_out = serf__bucket_http2_unframe_create(frame_in, FALSE, 16384, alloc); + frame_out = serf__bucket_http2_unframe_create(frame_in, 16384, alloc); read_and_check_bucket(tc, frame_out, "This is no config!");