diff mbox series

[Openvpn-devel,v3] buffer: Fix small issues in buf_printf and buf_puts

Message ID 20261017171858.16208-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v3] buffer: Fix small issues in buf_printf and buf_puts |

Commit Message

Gert Doering Oct. 17, 2026, 5:18 p.m. UTC 
  From: Frank Lichtenheld <frank@lichtenheld.com>

- 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 <imcusg@gmail.com>.

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 <frank@lichtenheld.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1997
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1997
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
diff mbox series

Patch

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),