[Openvpn-devel,v1] dns: clone options via pointer instead of copy

Message ID 20241213164630.266045-1-frank@lichtenheld.com
State Accepted
Headers show
Series [Openvpn-devel,v1] dns: clone options via pointer instead of copy | expand

Commit Message

Frank Lichtenheld Dec. 13, 2024, 4:46 p.m. UTC
From: Heiko Hund <heiko@ist.eigentlich.net>

Change-Id: I12b8bb26c0cb70e50b2d42b1c69018894e9f080c
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
---

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

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

Comments

Gert Doering Dec. 21, 2024, 3:29 p.m. UTC | #1
It's not immediately obvious to me why this is a useful change (except
"do not copy structs around if not needed"), but the actual change looks
good and should not change operational behaviour in any way - no memory
handling or anything involved, just "the midlayer" function referencing
structure elements via pointer now.

Compile tested :-)

Your patch has been applied to the master branch.

commit db46d4d38bec59435a76bb0968d530b9426c551e
Author: Heiko Hund
Date:   Fri Dec 13 17:46:30 2024 +0100

     dns: clone options via pointer instead of copy

     Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20241213164630.266045-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30112.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 15e7322..e22ea00 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -248,13 +248,15 @@ 
 }
 
 struct dns_options
-clone_dns_options(const struct dns_options o, struct gc_arena *gc)
+clone_dns_options(const struct dns_options *o, struct gc_arena *gc)
 {
     struct dns_options clone;
+
     memset(&clone, 0, sizeof(clone));
-    clone.search_domains = clone_dns_domains(o.search_domains, gc);
-    clone.servers = clone_dns_servers(o.servers, gc);
-    clone.servers_prepull = clone_dns_servers(o.servers_prepull, gc);
+    clone.search_domains = clone_dns_domains(o->search_domains, gc);
+    clone.servers = clone_dns_servers(o->servers, gc);
+    clone.servers_prepull = clone_dns_servers(o->servers_prepull, gc);
+
     return clone;
 }
 
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index a4ccb95..838ebe1 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -129,7 +129,8 @@ 
  * @param   gc          Pointer to the gc_arena to use for the clone
  * @return              The dns_options clone
  */
-struct dns_options clone_dns_options(const struct dns_options o, struct gc_arena *gc);
+struct dns_options clone_dns_options(const struct dns_options *o,
+                                     struct gc_arena *gc);
 
 /**
  * Saves and resets the server options, so that pulled ones don't mix in.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fa22eca..cc723ca 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3311,7 +3311,7 @@ 
     o->pre_connect->route_default_gateway = o->route_default_gateway;
     o->pre_connect->route_ipv6_default_gateway = o->route_ipv6_default_gateway;
 
-    o->pre_connect->dns_options = clone_dns_options(o->dns_options, &o->gc);
+    o->pre_connect->dns_options = clone_dns_options(&o->dns_options, &o->gc);
 
     /* NCP related options that can be overwritten by a push */
     o->pre_connect->ciphername = o->ciphername;
@@ -3364,7 +3364,7 @@ 
         /* Free DNS options and reset them to pre-pull state */
         gc_free(&o->dns_options.gc);
         struct gc_arena dns_gc = gc_new();
-        o->dns_options = clone_dns_options(pp->dns_options, &dns_gc);
+        o->dns_options = clone_dns_options(&pp->dns_options, &dns_gc);
         o->dns_options.gc = dns_gc;
 
         if (pp->client_nat_defined)