[Openvpn-devel,01/21] Remove max_size from buffer_list_new

Message ID 20211207170211.3275837-2-arne@rfc2549.org
State Accepted
Headers show
Series
  • Big buffer/frame refactoring patch set
Related show

Commit Message

Arne Schwabe Dec. 7, 2021, 5:01 p.m.
This argument is never used apart from a unit test. Remove this
argument as a small cleanup.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/buffer.c                   |  7 +++----
 src/openvpn/buffer.h                   |  4 +---
 src/openvpn/manage.c                   |  4 ++--
 src/openvpn/ssl.c                      |  2 +-
 tests/unit_tests/openvpn/test_buffer.c | 22 ++++------------------
 5 files changed, 11 insertions(+), 28 deletions(-)

Comments

Gert Doering Dec. 7, 2021, 6:50 p.m. | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

That was one of the easy ones :-) - verified that indeed, this option
is never used in OpenVPN code.

I have client-side tested this ("make check"), just to be sure.

"void" has been added to prototype and function declaration...

Your patch has been applied to the master branch.

commit 5962f6267fcdf33f83f8de725751b5ef41e94f7c
Author: Arne Schwabe
Date:   Tue Dec 7 18:01:51 2021 +0100

     Remove max_size from buffer_list_new

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20211207170211.3275837-2-arne@rfc2549.org>
     URL: https://www.mail-archive.com/search?l=mid&q=20211207170211.3275837-2-arne@rfc2549.org
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering Dec. 7, 2021, 6:53 p.m. | #2
Hi,

On Tue, Dec 07, 2021 at 07:50:33PM +0100, Gert Doering wrote:
> Acked-by: Gert Doering <gert@greenie.muc.de>
> 
> That was one of the easy ones :-) - verified that indeed, this option
> is never used in OpenVPN code.
> 
> I have client-side tested this ("make check"), just to be sure.
> 
> "void" has been added to prototype and function declaration...
> 
> Your patch has been applied to the master branch.
> 
> commit 5962f6267fcdf33f83f8de725751b5ef41e94f7c
> Author: Arne Schwabe
> Date:   Tue Dec 7 18:01:51 2021 +0100
> 
>      Remove max_size from buffer_list_new
> 
>      Signed-off-by: Arne Schwabe <arne@rfc2549.org>
>      Acked-by: Gert Doering <gert@greenie.muc.de>
>      Message-Id: <20211207170211.3275837-2-arne@rfc2549.org>
>      URL: https://www.mail-archive.com/search?l=mid&q=20211207170211.3275837-2-arne@rfc2549.org

*sigh* one day I'll learn... mail-archive search engine takes a few
hours before message IDs can be found.  So usually when a patch is
very fresh, I check the "URL:" that the script puts into the commit
message, and adjust from the web archive.

Forgot for this one, but noticed before pushing - so, fixing the commit
message with

  URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23329.html

which of course creates a new commit ID.

Thus:

commit 61d2f918d53c932acd1061b8523c54c18ebb8176 (HEAD -> master)
Author: Arne Schwabe <arne@rfc2549.org>
Date:   Tue Dec 7 18:01:51 2021 +0100

    Remove max_size from buffer_list_new

    Signed-off-by: Arne Schwabe <arne@rfc2549.org>
    Acked-by: Gert Doering <gert@greenie.muc.de>
    Message-Id: <20211207170211.3275837-2-arne@rfc2549.org>
    URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23329.html
    Signed-off-by: Gert Doering <gert@greenie.muc.de>


Stupid toolchain...

gert

Patch

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 486a77548..e9afb6d6a 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -1171,11 +1171,10 @@  valign4(const struct buffer *buf, const char *file, const int line)
  * struct buffer_list
  */
 struct buffer_list *
-buffer_list_new(const int max_size)
+buffer_list_new()
 {
     struct buffer_list *ret;
     ALLOC_OBJ_CLEAR(ret, struct buffer_list);
-    ret->max_size = max_size;
     ret->size = 0;
     return ret;
 }
@@ -1229,7 +1228,7 @@  struct buffer_entry *
 buffer_list_push_data(struct buffer_list *ol, const void *data, size_t size)
 {
     struct buffer_entry *e = NULL;
-    if (data && (!ol->max_size || ol->size < ol->max_size))
+    if (data)
     {
         ALLOC_OBJ_CLEAR(e, struct buffer_entry);
 
@@ -1359,7 +1358,7 @@  buffer_list_file(const char *fn, int max_line_len)
         char *line = (char *) malloc(max_line_len);
         if (line)
         {
-            bl = buffer_list_new(0);
+            bl = buffer_list_new();
             while (fgets(line, max_line_len, fp) != NULL)
             {
                 buffer_list_push(bl, line);
diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index 8cc03c08f..619c3a95d 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -1102,11 +1102,9 @@  struct buffer_list
 /**
  * Allocate an empty buffer list of capacity \c max_size.
  *
- * @param max_size  the capacity of the list to allocate
- *
  * @return the new list
  */
-struct buffer_list *buffer_list_new(const int max_size);
+struct buffer_list *buffer_list_new();
 
 /**
  * Frees a buffer list and all the buffers in it.
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 28315b82a..1f408f0b5 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -878,7 +878,7 @@  in_extra_reset(struct man_connection *mc, const int mode)
         }
         if (mode == IER_NEW)
         {
-            mc->in_extra = buffer_list_new(0);
+            mc->in_extra = buffer_list_new();
         }
     }
 }
@@ -2507,7 +2507,7 @@  man_connection_init(struct management *man)
          * command output from/to the socket.
          */
         man->connection.in = command_line_new(1024);
-        man->connection.out = buffer_list_new(0);
+        man->connection.out = buffer_list_new();
 
         /*
          * Initialize event set for standalone usage, when we are
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 0d811f24e..05096ee0a 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3989,7 +3989,7 @@  tls_send_payload(struct tls_multi *multi,
     {
         if (!ks->paybuf)
         {
-            ks->paybuf = buffer_list_new(0);
+            ks->paybuf = buffer_list_new();
         }
         buffer_list_push_data(ks->paybuf, data, (size_t)size);
         ret = true;
diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c
index 5e854c22e..ac701669f 100644
--- a/tests/unit_tests/openvpn/test_buffer.c
+++ b/tests/unit_tests/openvpn/test_buffer.c
@@ -67,18 +67,18 @@  static int
 test_buffer_list_setup(void **state)
 {
     struct test_buffer_list_aggregate_ctx *ctx  = calloc(1, sizeof(*ctx));
-    ctx->empty = buffer_list_new(0);
+    ctx->empty = buffer_list_new();
 
-    ctx->one_two_three = buffer_list_new(3);
+    ctx->one_two_three = buffer_list_new();
     buffer_list_push(ctx->one_two_three, teststr1);
     buffer_list_push(ctx->one_two_three, teststr2);
     buffer_list_push(ctx->one_two_three, teststr3);
 
-    ctx->zero_length_strings = buffer_list_new(2);
+    ctx->zero_length_strings = buffer_list_new();
     buffer_list_push(ctx->zero_length_strings, "");
     buffer_list_push(ctx->zero_length_strings, "");
 
-    ctx->empty_buffers = buffer_list_new(2);
+    ctx->empty_buffers = buffer_list_new();
     uint8_t data = 0;
     buffer_list_push_data(ctx->empty_buffers, &data, 0);
     buffer_list_push_data(ctx->empty_buffers, &data, 0);
@@ -100,17 +100,6 @@  test_buffer_list_teardown(void **state)
     return 0;
 }
 
-static void
-test_buffer_list_full(void **state)
-{
-    struct test_buffer_list_aggregate_ctx *ctx = *state;
-
-    /* list full */
-    assert_int_equal(ctx->one_two_three->size, 3);
-    buffer_list_push(ctx->one_two_three, teststr4);
-    assert_int_equal(ctx->one_two_three->size, 3);
-}
-
 static void
 test_buffer_list_aggregate_separator_empty(void **state)
 {
@@ -247,9 +236,6 @@  main(void)
 {
     const struct CMUnitTest tests[] = {
         cmocka_unit_test(test_buffer_strprefix),
-        cmocka_unit_test_setup_teardown(test_buffer_list_full,
-                                        test_buffer_list_setup,
-                                        test_buffer_list_teardown),
         cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_empty,
                                         test_buffer_list_setup,
                                         test_buffer_list_teardown),