From 15440d8027cdbc5557e6c71a389e1aa69d2b6f50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 30 Oct 2018 13:33:25 +0100 Subject: [PATCH 1/3] Add unit tests for isc_buffer_copyregion() Add some basic checks for isc_buffer_copyregion() to ensure it behaves as expected for both fixed-size buffers and buffers which can be automatically reallocated. Adjust the list of headers included by lib/isc/tests/buffer_test.c so that it matches what that test program really uses. --- lib/isc/tests/buffer_test.c | 55 +++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/lib/isc/tests/buffer_test.c b/lib/isc/tests/buffer_test.c index 34145d7bcc..89009e0a9d 100644 --- a/lib/isc/tests/buffer_test.c +++ b/lib/isc/tests/buffer_test.c @@ -10,17 +10,20 @@ */ #include + +#include +#include #include -#include -#include +#include #include #include "isctest.h" #include -#include +#include #include +#include ATF_TC(isc_buffer_reserve); ATF_TC_HEAD(isc_buffer_reserve, tc) { @@ -158,6 +161,51 @@ ATF_TC_BODY(isc_buffer_dynamic, tc) { isc_test_end(); } +ATF_TC(isc_buffer_copyregion); +ATF_TC_HEAD(isc_buffer_copyregion, tc) { + atf_tc_set_md_var(tc, "descr", "copy a region into a buffer"); +} + +ATF_TC_BODY(isc_buffer_copyregion, tc) { + unsigned char data[] = { 0x11, 0x22, 0x33, 0x44 }; + isc_buffer_t *b = NULL; + isc_result_t result; + + isc_region_t r = { + .base = data, + .length = sizeof(data), + }; + + result = isc_test_begin(NULL, true, 0); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = isc_buffer_allocate(mctx, &b, sizeof(data)); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* + * Fill originally allocated buffer space. + */ + result = isc_buffer_copyregion(b, &r); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + + /* + * Appending more data to the buffer should fail. + */ + result = isc_buffer_copyregion(b, &r); + ATF_CHECK_EQ(result, ISC_R_NOSPACE); + + /* + * Enable auto reallocation and retry. Appending should now succeed. + */ + isc_buffer_setautorealloc(b, true); + result = isc_buffer_copyregion(b, &r); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + + isc_buffer_free(&b); + + isc_test_end(); +} + ATF_TC(isc_buffer_printf); ATF_TC_HEAD(isc_buffer_printf, tc) { atf_tc_set_md_var(tc, "descr", "printf() into a buffer"); @@ -278,6 +326,7 @@ ATF_TC_BODY(isc_buffer_printf, tc) { ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, isc_buffer_reserve); ATF_TP_ADD_TC(tp, isc_buffer_dynamic); + ATF_TP_ADD_TC(tp, isc_buffer_copyregion); ATF_TP_ADD_TC(tp, isc_buffer_printf); return (atf_no_error()); } From e1f0aed03434e9f49208da8c7f617855665d6bca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 30 Oct 2018 13:33:25 +0100 Subject: [PATCH 2/3] Fix isc_buffer_copyregion() for auto-reallocated buffers While isc_buffer_copyregion() calls isc_buffer_reserve() to ensure the target buffer will have enough available space to append the contents of the source region to it, the variables used for subsequently checking available space are not updated accordingly after that call. This prevents isc_buffer_copyregion() from working as expected for auto-reallocated buffers: ISC_R_NOSPACE will be returned if enough space is not already available in the target buffer before it is reallocated. Fix by calling isc_buffer_used() and isc_buffer_availablelength() directly instead of assigning their return values to local variables. --- lib/isc/buffer.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 5eb2620c70..987795be12 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -514,27 +514,24 @@ isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src) { isc_result_t isc_buffer_copyregion(isc_buffer_t *b, const isc_region_t *r) { - unsigned char *base; - unsigned int available; isc_result_t result; REQUIRE(ISC_BUFFER_VALID(b)); REQUIRE(r != NULL); - /* - * XXXDCL - */ - base = isc_buffer_used(b); - available = isc_buffer_availablelength(b); if (ISC_UNLIKELY(b->autore)) { result = isc_buffer_reserve(&b, r->length); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { return (result); + } } - if (r->length > available) + + if (r->length > isc_buffer_availablelength(b)) { return (ISC_R_NOSPACE); + } + if (r->length > 0U) { - memmove(base, r->base, r->length); + memmove(isc_buffer_used(b), r->base, r->length); b->used += r->length; } From 07050fb49abcd86ec04242fbc6e0be5a9dc32121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 30 Oct 2018 13:33:25 +0100 Subject: [PATCH 3/3] Add CHANGES entry 5072. [bug] Add unit tests for isc_buffer_copyregion() and fix its behavior for auto-reallocated buffers. [GL #644] --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index afcce93836..8e6d993723 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5072. [bug] Add unit tests for isc_buffer_copyregion() and fix its + behavior for auto-reallocated buffers. [GL #644] + 5071. [bug] Comparision of NXT records was broken. [GL #631] 5070. [bug] Record types which support a empty rdata field were