Attention is currently required from: Arne Schwabe, Frank Lichtenheld, Răzvan
Cojocaru.
Hello Arne Schwabe, Răzvan Cojocaru,
I'd like you to reexamine a change. Please visit
https://gerrit.openvpn.net/c/openvpn/+/1997?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by Răzvan Cojocaru
Change subject: buffer: Fix small issues in buf_printf and buf_puts
......................................................................
buffer: Fix small issues in buf_printf and buf_puts
- Remove unused and wrongly document return value
of buf_puts
- Add UTs for buf_puts
Both of these changes based on a PR by
cuishuang <[email protected]>.
In both functions:
- Remove obsolete manual adding of \0
- Do not try to append if we can only overwrite
the trailing \0 byte
Change-Id: I1d6ed5d40910359b65b9f5c5d8f56ad3d3bc827a
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/buffer.c
M src/openvpn/buffer.h
M tests/unit_tests/openvpn/test_buffer.c
3 files changed, 39 insertions(+), 16 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/97/1997/2
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index cc1b300..3599cc9 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -225,21 +225,20 @@
bool
buf_printf(struct buffer *buf, const char *format, ...)
{
- int ret = false;
+ bool ret = false;
if (buf_defined(buf))
{
- uint8_t *ptr = BEND(buf);
+ char *ptr = (char *)BEND(buf);
int cap = buf_forward_capacity(buf);
- if (cap > 0)
+ if (cap > 1) /* can write more than \0 */
{
int stat;
va_list arglist;
va_start(arglist, format);
- stat = vsnprintf((char *)ptr, cap, format, arglist);
+ stat = vsnprintf(ptr, cap, format, arglist);
va_end(arglist);
- *(buf->data + buf->capacity - 1) = 0; /* windows vsnprintf needs
this */
- buf->len += (int)strlen((char *)ptr);
+ buf->len += (int)strlen(ptr);
if (stat >= 0 && stat < cap)
{
ret = true;
@@ -249,19 +248,16 @@
return ret;
}
-bool
+void
buf_puts(struct buffer *buf, const char *str)
{
- int ret = false;
- uint8_t *ptr = BEND(buf);
+ char *ptr = (char *)BEND(buf);
int cap = buf_forward_capacity(buf);
- if (cap > 0)
+ if (cap > 1) /* can write more than \0 */
{
- int str_len = (int)strlcpy((char *)ptr, str, cap);
+ int str_len = (int)strlcpy(ptr, str, cap);
buf->len += (str_len < cap) ? str_len : cap - 1;
- ret = true;
}
- return ret;
}
/*
diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index c66ed1a..eb64446 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -791,10 +791,8 @@
*
* @param buf The buffer to append to.
* @param str The null-terminated string to append.
- *
- * @return true if the string fit in the buffer, false if it was truncated.
*/
-bool buf_puts(struct buffer *buf, const char *str);
+void buf_puts(struct buffer *buf, const char *str);
/*
diff --git a/tests/unit_tests/openvpn/test_buffer.c
b/tests/unit_tests/openvpn/test_buffer.c
index bf486e0..338265d 100644
--- a/tests/unit_tests/openvpn/test_buffer.c
+++ b/tests/unit_tests/openvpn/test_buffer.c
@@ -62,6 +62,10 @@
buf_printf(&buf, "%d", 123);
buf_printf(&buf, "%s", "some text, too long to fit");
assert_buf_equals_str(&buf, "123some text, t");
+ assert_int_equal(BCAP(&buf), 1); /* \0 byte not part of len */
+
+ buf_printf(&buf, "%s", "more");
+ assert_buf_equals_str(&buf, "123some text, t");
buf_catrunc(&buf, "...");
assert_buf_equals_str(&buf, "123some text...");
@@ -79,6 +83,30 @@
}
static void
+test_buffer_puts(void **state)
+{
+ struct gc_arena gc = gc_new();
+ struct buffer buf = alloc_buf_gc(5, &gc);
+
+ buf_puts(&buf, "1234");
+ assert_string_equal(BSTR(&buf), "1234");
+
+ buf_reset_len(&buf);
+ buf_puts(&buf, "12345");
+ assert_string_equal(BSTR(&buf), "1234");
+
+ /* buf already full */
+ buf_puts(&buf, "12345");
+ assert_string_equal(BSTR(&buf), "1234");
+
+ struct buffer one_byte_buf = alloc_buf_gc(1, &gc);
+ buf_puts(&one_byte_buf, "x");
+ assert_string_equal(BSTR(&one_byte_buf), "");
+
+ gc_free(&gc);
+}
+
+static void
test_buffer_format_hex_ex(void **state)
{
const int input_size = 10;
@@ -632,6 +660,7 @@
const struct CMUnitTest tests[] = {
cmocka_unit_test(test_buffer_strprefix),
cmocka_unit_test(test_buffer_printf_catrunc),
+ cmocka_unit_test(test_buffer_puts),
cmocka_unit_test(test_buffer_format_hex_ex),
cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_empty,
test_buffer_list_setup,
test_buffer_list_teardown),
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/1997?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1d6ed5d40910359b65b9f5c5d8f56ad3d3bc827a
Gerrit-Change-Number: 1997
Gerrit-PatchSet: 2
Gerrit-Owner: Frank Lichtenheld <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-Reviewer: Răzvan Cojocaru <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Răzvan Cojocaru <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
Gerrit-Attention: Frank Lichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
