Skip to content

Commit 7d326e6

Browse files
flichtenheldcron2
authored andcommitted
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 <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 Message-Id: <20261017171858.16208-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg39947.html Signed-off-by: Gert Doering <gert@greenie.muc.de>
1 parent fa57eb1 commit 7d326e6

3 files changed

Lines changed: 39 additions & 16 deletions

File tree

‎src/openvpn/buffer.c‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -225,21 +225,20 @@ buf_sub(struct buffer *buf, int size, bool prepend)
225225
bool
226226
buf_printf(struct buffer *buf, const char *format, ...)
227227
{
228-
int ret = false;
228+
bool ret = false;
229229
if (buf_defined(buf))
230230
{
231-
uint8_t *ptr = BEND(buf);
231+
char *ptr = (char *)BEND(buf);
232232
int cap = buf_forward_capacity(buf);
233233

234-
if (cap > 0)
234+
if (cap > 1) /* can write more than \0 */
235235
{
236236
int stat;
237237
va_list arglist;
238238
va_start(arglist, format);
239-
stat = vsnprintf((char *)ptr, cap, format, arglist);
239+
stat = vsnprintf(ptr, cap, format, arglist);
240240
va_end(arglist);
241-
*(buf->data + buf->capacity - 1) = 0; /* windows vsnprintf needs this */
242-
buf->len += (int)strlen((char *)ptr);
241+
buf->len += (int)strlen(ptr);
243242
if (stat >= 0 && stat < cap)
244243
{
245244
ret = true;
@@ -249,19 +248,16 @@ buf_printf(struct buffer *buf, const char *format, ...)
249248
return ret;
250249
}
251250

252-
bool
251+
void
253252
buf_puts(struct buffer *buf, const char *str)
254253
{
255-
int ret = false;
256-
uint8_t *ptr = BEND(buf);
254+
char *ptr = (char *)BEND(buf);
257255
int cap = buf_forward_capacity(buf);
258-
if (cap > 0)
256+
if (cap > 1) /* can write more than \0 */
259257
{
260-
int str_len = (int)strlcpy((char *)ptr, str, cap);
258+
int str_len = (int)strlcpy(ptr, str, cap);
261259
buf->len += (str_len < cap) ? str_len : cap - 1;
262-
ret = true;
263260
}
264-
return ret;
265261
}
266262

267263
/*

‎src/openvpn/buffer.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -791,10 +791,8 @@ bool buf_printf(struct buffer *buf, const char *format, ...)
791791
*
792792
* @param buf The buffer to append to.
793793
* @param str The null-terminated string to append.
794-
*
795-
* @return true if the string fit in the buffer, false if it was truncated.
796794
*/
797-
bool buf_puts(struct buffer *buf, const char *str);
795+
void buf_puts(struct buffer *buf, const char *str);
798796

799797

800798
/*

‎tests/unit_tests/openvpn/test_buffer.c‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ test_buffer_printf_catrunc(void **state)
6262
buf_printf(&buf, "%d", 123);
6363
buf_printf(&buf, "%s", "some text, too long to fit");
6464
assert_buf_equals_str(&buf, "123some text, t");
65+
assert_int_equal(BCAP(&buf), 1); /* \0 byte not part of len */
66+
67+
buf_printf(&buf, "%s", "more");
68+
assert_buf_equals_str(&buf, "123some text, t");
6569

6670
buf_catrunc(&buf, "...");
6771
assert_buf_equals_str(&buf, "123some text...");
@@ -78,6 +82,30 @@ test_buffer_printf_catrunc(void **state)
7882
gc_free(&gc);
7983
}
8084

85+
static void
86+
test_buffer_puts(void **state)
87+
{
88+
struct gc_arena gc = gc_new();
89+
struct buffer buf = alloc_buf_gc(5, &gc);
90+
91+
buf_puts(&buf, "1234");
92+
assert_string_equal(BSTR(&buf), "1234");
93+
94+
buf_reset_len(&buf);
95+
buf_puts(&buf, "12345");
96+
assert_string_equal(BSTR(&buf), "1234");
97+
98+
/* buf already full */
99+
buf_puts(&buf, "12345");
100+
assert_string_equal(BSTR(&buf), "1234");
101+
102+
struct buffer one_byte_buf = alloc_buf_gc(1, &gc);
103+
buf_puts(&one_byte_buf, "x");
104+
assert_string_equal(BSTR(&one_byte_buf), "");
105+
106+
gc_free(&gc);
107+
}
108+
81109
static void
82110
test_buffer_format_hex_ex(void **state)
83111
{
@@ -632,6 +660,7 @@ main(void)
632660
const struct CMUnitTest tests[] = {
633661
cmocka_unit_test(test_buffer_strprefix),
634662
cmocka_unit_test(test_buffer_printf_catrunc),
663+
cmocka_unit_test(test_buffer_puts),
635664
cmocka_unit_test(test_buffer_format_hex_ex),
636665
cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_empty,
637666
test_buffer_list_setup, test_buffer_list_teardown),

0 commit comments

Comments
 (0)