diff mbox series

[Openvpn-devel,v1] buffer: restructure some loops and assignments

Message ID 20261027130252.2457-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v1] buffer: restructure some loops and assignments |

Commit Message

Gert Doering Oct. 27, 2026, 1:02 p.m. UTC 
  From: Jeff Bindel <jeff@incrediblybased.co>

Small cleanups in buffer.c that do not change behaviour, in preparation
for building it with -fbounds-safety:

- alloc_buf(), alloc_buf_gc(), clone_buf(): assign buf.data before
  buf.capacity and drop the redundant casts of the allocator result
- free_buf_gc(): compare the entry address arithmetically instead of
  stepping a struct gc_entry pointer past its object
- buf_printf(): bound the length of the written string by the capacity
  passed to vsnprintf()
- string_array_len(), print_argv(), string_class(), string_substitute():
  walk NUL-terminated strings and arrays by advancing the pointer
- string_mod(): write through a separate output pointer
- string_alloc_buf(): use a temporary for the copied string
- buf_parse(): check for an empty line without strlen()

Change-Id: Icf890b018b9950c698e46cc44b39edaa0c6ec566
Signed-off-by: Jeff Bindel <jeff@incrediblybased.co>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2034
---

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/+/2034
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>
diff mbox series

Patch

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index c683f8e..8813fa5 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -66,8 +66,8 @@ 
     {
         buf_size_error(size);
     }
-    buf.capacity = (int)size;
     buf.data = calloc(1, size);
+    buf.capacity = (int)size;
     check_malloc_return(buf.data);
 
     return buf;
@@ -83,8 +83,8 @@ 
     {
         buf_size_error(size);
     }
+    buf.data = gc_malloc(size, false, gc);
     buf.capacity = (int)size;
-    buf.data = (uint8_t *)gc_malloc(size, false, gc);
     if (size)
     {
         *buf.data = 0;
@@ -100,15 +100,15 @@ 
 #endif
 {
     struct buffer ret;
+    ret.data = malloc(buf->capacity);
     ret.capacity = buf->capacity;
+    check_malloc_return(ret.data);
     ret.offset = buf->offset;
     ret.len = buf->len;
 #ifdef BUF_INIT_TRACKING
     ret.debug_file = buf->debug_file;
     ret.debug_line = buf->debug_line;
 #endif
-    ret.data = (uint8_t *)malloc(buf->capacity);
-    check_malloc_return(ret.data);
     memcpy(BPTR(&ret), CBPTR(buf), BLENZ(buf));
     return ret;
 }
@@ -182,7 +182,7 @@ 
         while (*e)
         {
             /* check if this object is the one we want to delete */
-            if ((uint8_t *)(*e + 1) == buf->data)
+            if ((uintptr_t)*e + sizeof(struct gc_entry) == (uintptr_t)buf->data)
             {
                 struct gc_entry *to_delete = *e;
 
@@ -238,7 +238,7 @@ 
             va_start(arglist, format);
             stat = vsnprintf(ptr, cap, format, arglist);
             va_end(arglist);
-            buf->len += (int)strlen(ptr);
+            buf->len += (int)strnlen(ptr, cap);
             if (stat >= 0 && stat < cap)
             {
                 ret = true;
@@ -607,7 +607,7 @@ 
 
         if (gc)
         {
-            ret = (char *)gc_malloc(n, false, gc);
+            ret = gc_malloc(n, false, gc);
         }
         else
         {
@@ -648,7 +648,7 @@ 
     int i = 0;
     if (array)
     {
-        while (array[i])
+        for (; *array; ++array)
         {
             ++i;
         }
@@ -661,13 +661,9 @@ 
 {
     struct buffer out = alloc_buf_gc(256, gc);
     int i = 0;
-    for (;;)
+    for (; *p; ++p)
     {
-        const char *cp = *p++;
-        if (!cp)
-        {
-            break;
-        }
+        const char *cp = *p;
         if (i)
         {
             buf_printf(&out, " ");
@@ -695,7 +691,8 @@ 
 
     ASSERT(str);
 
-    buf_set_read(&buf, (uint8_t *)string_alloc(str, gc), strlen(str) + 1);
+    const char *copy = string_alloc(str, gc);
+    buf_set_read(&buf, (const uint8_t *)copy, strlen(str) + 1);
 
     if (buf.len > 0) /* Don't count trailing '\0' as part of length */
     {
@@ -789,7 +786,7 @@ 
     } while (c);
 
     line[size - 1] = '\0';
-    return !(eol && !strlen(line));
+    return !(eol && line[0] == '\0');
 }
 
 /*
@@ -970,11 +967,10 @@ 
 bool
 string_class(const char *str, const unsigned int inclusive, const unsigned int exclusive)
 {
-    char c;
     ASSERT(str);
-    while ((c = *str++))
+    for (; *str; ++str)
     {
-        if (!char_inc_exc(c, inclusive, exclusive))
+        if (!char_inc_exc(*str, inclusive, exclusive))
         {
             return false;
         }
@@ -990,11 +986,12 @@ 
 string_mod(char *str, const unsigned int inclusive, const unsigned int exclusive,
            const char replace)
 {
-    const char *in = str;
-    bool ret = true;
-
     ASSERT(str);
 
+    char *out = str;
+    const char *in = out;
+    bool ret = true;
+
     while (true)
     {
         char c = *in++;
@@ -1007,12 +1004,12 @@ 
             }
             if (c)
             {
-                *str++ = c;
+                *out++ = c;
             }
         }
         else
         {
-            *str = '\0';
+            *out = '\0';
             break;
         }
     }
@@ -1103,19 +1100,14 @@ 
 char *
 string_substitute(const char *src, char from, char to, struct gc_arena *gc)
 {
-    char *ret = (char *)gc_malloc(strlen(src) + 1, true, gc);
+    char *ret = gc_malloc(strlen(src) + 1, true, gc);
     char *dest = ret;
-    char c;
 
-    do
+    for (; *src; ++src)
     {
-        c = *src++;
-        if (c == from)
-        {
-            c = to;
-        }
-        *dest++ = c;
-    } while (c);
+        *dest++ = (*src == from) ? to : *src;
+    }
+    *dest = '\0';
     return ret;
 }