diff --git a/CHANGES b/CHANGES index 28e7a721e4..d6d9913315 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4767. [func] Add a new function, isc_buffer_printf(), which can be + used to append a formatted string to the used region of + a buffer. [RT #46201] + 4766. [cleanup] Addresss Coverity warnings. [RT #46150] 4765. [bug] Address potential INSIST in dnssec-cds. [RT #46150] diff --git a/bin/named/unix/os.c b/bin/named/unix/os.c index 7b2ae1e9f2..56f52c44bc 100644 --- a/bin/named/unix/os.c +++ b/bin/named/unix/os.c @@ -1036,12 +1036,7 @@ named_os_shutdownmsg(char *command, isc_buffer_t *text) { pid = getpid(); #endif - n = snprintf((char *)isc_buffer_used(text), - isc_buffer_availablelength(text), - "pid: %ld", (long)pid); - /* Only send a message if it is complete. */ - if (n > 0 && n < isc_buffer_availablelength(text)) - isc_buffer_add(text, n); + (void)isc_buffer_printf(text, "pid: %ld", (long)pid); } void diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 76212bf5aa..3a5de99b90 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -1875,15 +1875,8 @@ buildfilename(dns_name_t *name, dns_keytag_t id, result = dns_name_tofilenametext(name, ISC_FALSE, out); if (result != ISC_R_SUCCESS) return (result); - len = 1 + 3 + 1 + 5 + strlen(suffix) + 1; - if (isc_buffer_availablelength(out) < len) - return (ISC_R_NOSPACE); - snprintf((char *) isc_buffer_used(out), - (int)isc_buffer_availablelength(out), - "+%03d+%05d%s", alg, id, suffix); - isc_buffer_add(out, len); - return (ISC_R_SUCCESS); + return (isc_buffer_printf(out, "+%03d+%05d%s", alg, id, suffix)); } static isc_result_t diff --git a/lib/isc/buffer.c b/lib/isc/buffer.c index 95b5c5ec28..8c635da785 100644 --- a/lib/isc/buffer.c +++ b/lib/isc/buffer.c @@ -17,6 +17,8 @@ #include #include +#include + void isc__buffer_init(isc_buffer_t *b, void *base, unsigned int length) { /* @@ -633,3 +635,37 @@ isc_buffer_free(isc_buffer_t **dynbuffer) { isc_buffer_invalidate(dbuf); isc_mem_put(mctx, dbuf, sizeof(isc_buffer_t)); } + +isc_result_t +isc_buffer_printf(isc_buffer_t *b, const char *format, ...) { + va_list ap; + int n; + isc_result_t result; + + REQUIRE(ISC_BUFFER_VALID(b)); + + va_start(ap, format); + n = vsnprintf(NULL, 0, format, ap); + va_end(ap); + + if (n < 0) { + return (ISC_R_FAILURE); + } + + result = isc_buffer_reserve(&b, n + 1); + if (result != ISC_R_SUCCESS) { + return (result); + } + + va_start(ap, format); + n = vsnprintf(isc_buffer_used(b), n + 1, format, ap); + va_end(ap); + + if (n < 0) { + return (ISC_R_FAILURE); + } + + b->used += n; + + return (ISC_R_SUCCESS); +} diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 9cb3388973..b8971a4a57 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -1046,13 +1046,9 @@ isc_httpd_response(isc_httpd_t *httpd) { return (result); } - snprintf(isc_buffer_used(&httpd->headerbuffer), - (int)isc_buffer_availablelength(&httpd->headerbuffer), - "%s %03u %s\r\n", httpd->protocol, httpd->retcode, - httpd->retmsg); - isc_buffer_add(&httpd->headerbuffer, needlen); - - return (ISC_R_SUCCESS); + return (isc_buffer_printf(&httpd->headerbuffer, "%s %03u %s\r\n", + httpd->protocol, httpd->retcode, + httpd->retmsg)); } isc_result_t @@ -1073,18 +1069,13 @@ isc_httpd_addheader(isc_httpd_t *httpd, const char *name, return (result); } - if (val != NULL) - snprintf(isc_buffer_used(&httpd->headerbuffer), - isc_buffer_availablelength(&httpd->headerbuffer), - "%s: %s\r\n", name, val); - else - snprintf(isc_buffer_used(&httpd->headerbuffer), - isc_buffer_availablelength(&httpd->headerbuffer), - "%s\r\n", name); - - isc_buffer_add(&httpd->headerbuffer, needlen); - - return (ISC_R_SUCCESS); + if (val != NULL) { + return (isc_buffer_printf(&httpd->headerbuffer, "%s: %s\r\n", + name, val)); + } else { + return (isc_buffer_printf(&httpd->headerbuffer, "%s\r\n", + name)); + } } isc_result_t @@ -1097,11 +1088,7 @@ isc_httpd_endheaders(isc_httpd_t *httpd) { return (result); } - snprintf(isc_buffer_used(&httpd->headerbuffer), - isc_buffer_availablelength(&httpd->headerbuffer), "\r\n"); - isc_buffer_add(&httpd->headerbuffer, 2); - - return (ISC_R_SUCCESS); + return (isc_buffer_printf(&httpd->headerbuffer, "\r\n")); } isc_result_t @@ -1122,13 +1109,8 @@ isc_httpd_addheaderuint(isc_httpd_t *httpd, const char *name, int val) { return (result); } - snprintf(isc_buffer_used(&httpd->headerbuffer), - isc_buffer_availablelength(&httpd->headerbuffer), - "%s: %s\r\n", name, buf); - - isc_buffer_add(&httpd->headerbuffer, needlen); - - return (ISC_R_SUCCESS); + return (isc_buffer_printf(&httpd->headerbuffer, "%s: %s\r\n", name, + buf)); } static void diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 3ed8f204d9..ae9fc412b8 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -741,6 +741,38 @@ isc_buffer_dup(isc_mem_t *mctx, isc_buffer_t **dstp, const isc_buffer_t *src); * big enough. */ +isc_result_t +isc_buffer_printf(isc_buffer_t *b, const char *format, ...) + ISC_FORMAT_PRINTF(2, 3); +/*!< + * \brief Append a formatted string to the used region of 'b'. + * + * Notes: + * + *\li The 'format' argument is a printf(3) string, with additional arguments + * as necessary. + * + *\li If 'b' has autoreallocation enabled, and the formatted string + * would overrun the buffer, the buffer is reallocated. + * + * Requires: + * + *\li 'b' is a valid buffer. + * + * Ensures: + * + *\li The used pointer in 'b' is advanced by the number of bytes appended + * (excluding the terminating NULL byte). + * + * Returns: + * + *\li #ISC_R_SUCCESS Operation succeeded. + *\li #ISC_R_NOSPACE 'b' does not allow reallocation and appending the + * formatted string to it would cause it to overflow. + *\li #ISC_R_NOMEMORY Reallocation failed. + *\li #ISC_R_FAILURE Other error occurred. + */ + ISC_LANG_ENDDECLS /* diff --git a/lib/isc/tests/buffer_test.c b/lib/isc/tests/buffer_test.c index c9c36abc8b..0b3156ff87 100644 --- a/lib/isc/tests/buffer_test.c +++ b/lib/isc/tests/buffer_test.c @@ -189,6 +189,125 @@ ATF_TC_BODY(isc_buffer_dynamic, tc) { 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"); +} + +ATF_TC_BODY(isc_buffer_printf, tc) { + const char *bad_fmt, *empty_fmt; + unsigned int used, prev_used; + isc_result_t result; + isc_buffer_t *b, sb; + char buf[8]; + + result = isc_test_begin(NULL, ISC_TRUE); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* + * Prepare a buffer with auto-reallocation enabled. + */ + b = NULL; + result = isc_buffer_allocate(mctx, &b, 0); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + isc_buffer_setautorealloc(b, ISC_TRUE); + + /* + * Sanity check. + */ + result = isc_buffer_printf(b, "foo"); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(used, 3); + + result = isc_buffer_printf(b, "bar"); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(used, 3 + 3); + + /* + * Also check the terminating NULL byte is there, even though it is not + * part of the buffer's used region. + */ + ATF_CHECK_EQ(memcmp(isc_buffer_current(b), "foobar", 7), 0); + + /* + * Skip over data from previous check to prevent failures in previous + * check from affecting this one. + */ + prev_used = used; + isc_buffer_forward(b, prev_used); + + /* + * Some standard usage checks. + */ + isc_buffer_printf(b, "%d", 42); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(used - prev_used, 2); + + isc_buffer_printf(b, "baz%1X", 42); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(used - prev_used, 2 + 5); + + isc_buffer_printf(b, "%6.1f", 42.42f); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(used - prev_used, 2 + 5 + 6); + + /* + * Also check the terminating NULL byte is there, even though it is not + * part of the buffer's used region. + */ + ATF_CHECK_EQ(memcmp(isc_buffer_current(b), "42baz2A 42.4", 14), 0); + + /* + * Check an empty format string is properly handled. + */ + prev_used = used; + empty_fmt = ""; + result = isc_buffer_printf(b, empty_fmt, NULL); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(prev_used, used); + + /* + * Ensure vsnprintf() errors do not cause the buffer to be modified. + */ + prev_used = used; + bad_fmt = "%"; + result = isc_buffer_printf(b, bad_fmt, NULL); + ATF_CHECK_EQ(result, ISC_R_FAILURE); + used = isc_buffer_usedlength(b); + ATF_CHECK_EQ(prev_used, used); + + isc_buffer_free(&b); + + /* + * Check overflow on a static buffer. + */ + isc_buffer_init(&sb, buf, sizeof(buf)); + result = isc_buffer_printf(&sb, "123456"); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + used = isc_buffer_usedlength(&sb); + ATF_CHECK_EQ(used, 6); + + result = isc_buffer_printf(&sb, "789"); + ATF_CHECK_EQ(result, ISC_R_NOSPACE); + used = isc_buffer_usedlength(&sb); + ATF_CHECK_EQ(used, 6); + + result = isc_buffer_printf(&sb, "78"); + ATF_CHECK_EQ(result, ISC_R_NOSPACE); + used = isc_buffer_usedlength(&sb); + ATF_CHECK_EQ(used, 6); + + result = isc_buffer_printf(&sb, "7"); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + used = isc_buffer_usedlength(&sb); + ATF_CHECK_EQ(used, 7); + + isc_test_end(); +} + /* * Main */ @@ -196,5 +315,6 @@ ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, isc_buffer_reserve); ATF_TP_ADD_TC(tp, isc_buffer_reallocate); ATF_TP_ADD_TC(tp, isc_buffer_dynamic); + ATF_TP_ADD_TC(tp, isc_buffer_printf); return (atf_no_error()); }