[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

[its_Giaan (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]

Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email

to review the following change.


Change subject: Multi-socket: local_list clean-up
......................................................................

Multi-socket: local_list clean-up

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: #682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gianmarco@...2726...>
---
M src/openvpn/options.c
M src/openvpn/options.h
2 files changed, 14 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/1039/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..70337b1 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2212,12 +2212,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;

-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);

-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }

     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@

 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };

 struct connection_list

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gianmarco@...2726...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-MessageType: newchange

[-- Attachment #2: Type: text/html, Size: 5304 bytes --]

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

[cron2 (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

Attention is currently required from: flichtenheld, its_Giaan, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email )

Change subject: Multi-socket: local_list clean-up
......................................................................


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gianmarco@...2726...>
Gerrit-Reviewer: cron2 <gert@...1296...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Attention: its_Giaan <gianmarco@...2726...>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-Comment-Date: Wed, 18 Jun 2025 13:59:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2229 bytes --]

[Openvpn-devel] [PATCH v1] Multi-socket: local_list clean-up

[Gert Doering]

From: Gianmarco De Gregori <gianmarco@...2726...>

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: #682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gianmarco@...2726...>
Acked-by: Gert Doering <gert@...1296...>
---

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

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@...1296...>

        
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6ea01d4..70337b1 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2212,12 +2212,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;
 
-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);
 
-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }
 
     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@
 
 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };
 
 struct connection_list

[Openvpn-devel] [PATCH applied] Re: Multi-socket: local_list clean-up

[Gert Doering]

Stared-at-code, looks good.  Tested on the server instance with lots of
--local lines (+ some msg() calls to see l->len and l_capacity), and it
does what it promises.  Thanks :-)

Your patch has been applied to the master branch.

commit 9bb02bc34f5ecc85364fa7ab64e52b6c5c918055
Author: Gianmarco De Gregori
Date:   Wed Jun 18 16:00:09 2025 +0200

     Multi-socket: local_list clean-up

     Signed-off-by: Gianmarco De Gregori <gianmarco@...2726...>
     Acked-by: Gert Doering <gert@...1296...>
     Message-Id: <20250618140016.2766-1-gert@...1296...>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31927.html
     Signed-off-by: Gert Doering <gert@...1296...>


--
kind regards,

Gert Doering

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

[cron2 (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 3111 bytes --]

cron2 has uploaded a new patch set (#2) to the change originally created by its_Giaan. ( http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: Multi-socket: local_list clean-up
......................................................................

Multi-socket: local_list clean-up

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: OpenVPN/openvpn#682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gianmarco@...2726...>
Acked-by: Gert Doering <gert@...1296...>
Message-Id: <20250618140016.2766-1-gert@...1296...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31927.html
Signed-off-by: Gert Doering <gert@...1296...>
---
M src/openvpn/options.c
M src/openvpn/options.h
2 files changed, 14 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/39/1039/2

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d758a67..3cf8c2a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2067,12 +2067,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;

-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);

-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }

     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@

 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };

 struct connection_list

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 2
Gerrit-Owner: its_Giaan <gianmarco@...2726...>
Gerrit-Reviewer: cron2 <gert@...1296...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-MessageType: newpatchset

[-- Attachment #2: Type: text/html, Size: 5471 bytes --]

[Openvpn-devel] [S] Change in openvpn[master]: Multi-socket: local_list clean-up

[cron2 (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email )

Change subject: Multi-socket: local_list clean-up
......................................................................

Multi-socket: local_list clean-up

Optimize the current local_list implementation
by replacing the static array with a resizable
one, as the static allocation serves no real
purpose, particularly on the client side.

Github: OpenVPN/openvpn#682

Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Signed-off-by: Gianmarco De Gregori <gianmarco@...2726...>
Acked-by: Gert Doering <gert@...1296...>
Message-Id: <20250618140016.2766-1-gert@...1296...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31927.html
Signed-off-by: Gert Doering <gert@...1296...>
---
M src/openvpn/options.c
M src/openvpn/options.h
2 files changed, 14 insertions(+), 5 deletions(-)




diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d758a67..3cf8c2a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2067,12 +2067,20 @@
     struct local_list *l = alloc_local_list_if_undef(ce, gc);
     struct local_entry *e;

-    if (l->len >= CONNECTION_LIST_SIZE)
+    if (l->len >= l->capacity)
     {
-        msg(msglevel, "Maximum number of 'local' options (%d) exceeded",
-            CONNECTION_LIST_SIZE);
+        const int new_cap = l->capacity + 1;
+        const size_t elem_size = sizeof(*l->array);

-        return NULL;
+        struct local_entry **new_array = gc_realloc(l->array, new_cap * elem_size, gc);
+        if (!new_array)
+        {
+            msg(msglevel, "Unable to process more local options: out of memory. Number of entries = %d", l->len);
+            return NULL;
+        }
+
+        l->array = new_array;
+        l->capacity = new_cap;
     }

     ALLOC_OBJ_CLEAR_GC(e, struct local_entry, gc);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index b28ad58..46ec32b 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -188,8 +188,9 @@

 struct local_list
 {
+    int capacity;
     int len;
-    struct local_entry *array[CONNECTION_LIST_SIZE];
+    struct local_entry **array;
 };

 struct connection_list

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1039?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I32effed9e273fbe8986d1f4e8da4a4d0ac216463
Gerrit-Change-Number: 1039
Gerrit-PatchSet: 2
Gerrit-Owner: its_Giaan <gianmarco@...2726...>
Gerrit-Reviewer: cron2 <gert@...1296...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-Reviewer: plaisthos <arne-openvpn@...1227...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-MessageType: merged

[-- Attachment #2: Type: text/html, Size: 5232 bytes --]