From 4e65bc35ed1049b6becc0950fb50358504a4867d Mon Sep 17 00:00:00 2001 From: NiteHawk Date: Mon, 17 Aug 2015 15:59:19 +0200 Subject: [PATCH 1/4] refactor example/c/lib_buffer_unpack.c The example has some duplicated code that somewhat distracts from the main processing loop. I think placing this into a separate function improves readability of the code. --- example/c/lib_buffer_unpack.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/example/c/lib_buffer_unpack.c b/example/c/lib_buffer_unpack.c index a737f630..a143f9e6 100644 --- a/example/c/lib_buffer_unpack.c +++ b/example/c/lib_buffer_unpack.c @@ -41,6 +41,19 @@ size_t receiver_recv(receiver *r, char* buf, size_t try_size) { return actual_size; } +size_t receiver_to_unpacker(receiver* r, size_t request_size, + msgpack_unpacker *unpacker) +{ + // make sure there's enough room, or expand the unpacker accordingly + if (msgpack_unpacker_buffer_capacity(unpacker) < request_size) { + assert(msgpack_unpacker_reserve_buffer(unpacker, request_size)); + } + size_t recv_len = receiver_recv(r, msgpack_unpacker_buffer(unpacker), + request_size); + msgpack_unpacker_buffer_consumed(unpacker, recv_len); + return recv_len; +} + #define EACH_RECV_SIZE 4 void unpack(receiver* r) { @@ -54,17 +67,9 @@ void unpack(receiver* r) { int i = 0; msgpack_unpacked_init(&result); - if (msgpack_unpacker_buffer_capacity(unp) < EACH_RECV_SIZE) { - bool expanded = msgpack_unpacker_reserve_buffer(unp, 100); - assert(expanded); - } - buf = msgpack_unpacker_buffer(unp); - recv_len = receiver_recv(r, buf, EACH_RECV_SIZE); - msgpack_unpacker_buffer_consumed(unp, recv_len); - - - while (recv_len > 0) { + recv_len = receiver_to_unpacker(r, EACH_RECV_SIZE, unp); + while (recv_len > 0) { printf("receive count: %d %zd bytes received.\n", recv_count++, recv_len); ret = msgpack_unpacker_next(unp, &result); while (ret == MSGPACK_UNPACK_SUCCESS) { @@ -85,13 +90,7 @@ void unpack(receiver* r) { msgpack_unpacked_destroy(&result); return; } - if (msgpack_unpacker_buffer_capacity(unp) < EACH_RECV_SIZE) { - bool expanded = msgpack_unpacker_reserve_buffer(unp, 100); - assert(expanded); - } - buf = msgpack_unpacker_buffer(unp); - recv_len = receiver_recv(r, buf, 4); - msgpack_unpacker_buffer_consumed(unp, recv_len); + recv_len = receiver_to_unpacker(r, EACH_RECV_SIZE, unp); } msgpack_unpacked_destroy(&result); } From 871a7960375a0cc16695639923db734693a93cf9 Mon Sep 17 00:00:00 2001 From: NiteHawk Date: Mon, 17 Aug 2015 16:23:11 +0200 Subject: [PATCH 2/4] fix: remove unused variable introduced by previous commit --- example/c/lib_buffer_unpack.c | 1 - 1 file changed, 1 deletion(-) diff --git a/example/c/lib_buffer_unpack.c b/example/c/lib_buffer_unpack.c index a143f9e6..43686207 100644 --- a/example/c/lib_buffer_unpack.c +++ b/example/c/lib_buffer_unpack.c @@ -61,7 +61,6 @@ void unpack(receiver* r) { msgpack_unpacker* unp = msgpack_unpacker_new(100); msgpack_unpacked result; msgpack_unpack_return ret; - char* buf; size_t recv_len; int recv_count = 0; int i = 0; From 1788d6ce0112d903d04596191564973c61a4f1d3 Mon Sep 17 00:00:00 2001 From: NiteHawk Date: Mon, 17 Aug 2015 21:49:03 +0200 Subject: [PATCH 3/4] amend pull request (#344) following the related discussion There's a small problem remaining if assertions are disabled (with -DNDEBUG). --- example/c/lib_buffer_unpack.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/example/c/lib_buffer_unpack.c b/example/c/lib_buffer_unpack.c index 43686207..54ce70f9 100644 --- a/example/c/lib_buffer_unpack.c +++ b/example/c/lib_buffer_unpack.c @@ -46,7 +46,8 @@ size_t receiver_to_unpacker(receiver* r, size_t request_size, { // make sure there's enough room, or expand the unpacker accordingly if (msgpack_unpacker_buffer_capacity(unpacker) < request_size) { - assert(msgpack_unpacker_reserve_buffer(unpacker, request_size)); + bool expanded = msgpack_unpacker_reserve_buffer(unpacker, request_size); + assert(expanded); } size_t recv_len = receiver_recv(r, msgpack_unpacker_buffer(unpacker), request_size); @@ -66,9 +67,9 @@ void unpack(receiver* r) { int i = 0; msgpack_unpacked_init(&result); - - recv_len = receiver_to_unpacker(r, EACH_RECV_SIZE, unp); - while (recv_len > 0) { + while (true) { + recv_len = receiver_to_unpacker(r, EACH_RECV_SIZE, unp); + if (recv_len == 0) break; // (reached end of input) printf("receive count: %d %zd bytes received.\n", recv_count++, recv_len); ret = msgpack_unpacker_next(unp, &result); while (ret == MSGPACK_UNPACK_SUCCESS) { @@ -89,7 +90,6 @@ void unpack(receiver* r) { msgpack_unpacked_destroy(&result); return; } - recv_len = receiver_to_unpacker(r, EACH_RECV_SIZE, unp); } msgpack_unpacked_destroy(&result); } From 720c18bcf8b5a58680d7347cccb34e3813b4bdda Mon Sep 17 00:00:00 2001 From: NiteHawk Date: Tue, 18 Aug 2015 12:24:16 +0200 Subject: [PATCH 4/4] add missing msgpack_unpacker_free(), and a "clean" assertion avoiding side effects --- example/c/lib_buffer_unpack.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/example/c/lib_buffer_unpack.c b/example/c/lib_buffer_unpack.c index 54ce70f9..8abce921 100644 --- a/example/c/lib_buffer_unpack.c +++ b/example/c/lib_buffer_unpack.c @@ -46,8 +46,8 @@ size_t receiver_to_unpacker(receiver* r, size_t request_size, { // make sure there's enough room, or expand the unpacker accordingly if (msgpack_unpacker_buffer_capacity(unpacker) < request_size) { - bool expanded = msgpack_unpacker_reserve_buffer(unpacker, request_size); - assert(expanded); + msgpack_unpacker_reserve_buffer(unpacker, request_size); + assert(msgpack_unpacker_buffer_capacity(unpacker) >= request_size); } size_t recv_len = receiver_recv(r, msgpack_unpacker_buffer(unpacker), request_size); @@ -92,6 +92,7 @@ void unpack(receiver* r) { } } msgpack_unpacked_destroy(&result); + msgpack_unpacker_free(unp); } int main(void) {