[Openvpn-devel,v3,1/4] Simplify --compress parsing in options.c

Message ID 20230323170601.1256132-1-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v3,1/4] Simplify --compress parsing in options.c | expand

Commit Message

Arne Schwabe March 23, 2023, 5:05 p.m. UTC
This removes a level of identation and make the "stub" condition
easier to see.

Change-Id: Iae47b191f522625f81eedd3a237b272cb7374d90
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/options.c | 87 +++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 44 deletions(-)

Comments

Gert Doering March 23, 2023, 5:24 p.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

"git show -w" shows that this is mostly whitespace changes and
streq()'ing alg instead of p[1] - with alg defaulting to "stub"
now (instead of having an else{} clause for "no option" that does
the same).

There is a minor difference, as "compress <no args>" would set
COMP_F_SWAP beforehand, and now adds COMP_F_ADVERTISE_STUBS_ONLY,
which might warrant an update to the documentation (it says "Additionally,
'stub' and 'stub-v2' will disable announcing lzo and lz4 compression",
so there is a documented difference to 'empty').

Client-side tested on Linux / t_client with and without compression.

Your patch has been applied to the master and release/2.6 branch.

commit bfc00a01c10bbdd9683aab5db2c2e7dcbb2f7378 (master)
commit 5fed4be1bf4b2c6e4ff0117bceb9613fa68b412d (release/2.6)
Author: Arne Schwabe
Date:   Thu Mar 23 18:05:58 2023 +0100

     Simplify --compress parsing in options.c

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 64a8250b2..2bed4ce99 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8458,60 +8458,59 @@  add_option(struct options *options,
     else if (streq(p[0], "compress") && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_COMP);
+        const char *alg = "stub";
         if (p[1])
         {
-            if (streq(p[1], "stub"))
-            {
-                options->comp.alg = COMP_ALG_STUB;
-                options->comp.flags |= (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
-            }
-            else if (streq(p[1], "stub-v2"))
-            {
-                options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
-                options->comp.flags |= COMP_F_ADVERTISE_STUBS_ONLY;
-            }
-            else if (streq(p[1], "migrate"))
-            {
-                options->comp.alg = COMP_ALG_UNDEF;
-                options->comp.flags = COMP_F_MIGRATE;
+            alg = p[1];
+        }
 
-            }
-            else if (options->comp.flags & COMP_F_ALLOW_STUB_ONLY)
-            {
-                /* Also printed on a push to hint at configuration problems */
-                msg(msglevel, "Cannot set compress to '%s', "
-                    "allow-compression is set to 'no'", p[1]);
-                goto err;
-            }
+        if (streq(alg, "stub"))
+        {
+            options->comp.alg = COMP_ALG_STUB;
+            options->comp.flags |= (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
+        }
+        else if (streq(alg, "stub-v2"))
+        {
+            options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+            options->comp.flags |= COMP_F_ADVERTISE_STUBS_ONLY;
+        }
+        else if (streq(alg, "migrate"))
+        {
+            options->comp.alg = COMP_ALG_UNDEF;
+            options->comp.flags = COMP_F_MIGRATE;
+
+        }
+        else if (options->comp.flags & COMP_F_ALLOW_STUB_ONLY)
+        {
+            /* Also printed on a push to hint at configuration problems */
+            msg(msglevel, "Cannot set compress to '%s', "
+                "allow-compression is set to 'no'", alg);
+            goto err;
+        }
 #if defined(ENABLE_LZO)
-            else if (streq(p[1], "lzo"))
-            {
-                options->comp.alg = COMP_ALG_LZO;
-                options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
-            }
+        else if (streq(alg, "lzo"))
+        {
+            options->comp.alg = COMP_ALG_LZO;
+            options->comp.flags &= ~(COMP_F_ADAPTIVE | COMP_F_SWAP);
+        }
 #endif
 #if defined(ENABLE_LZ4)
-            else if (streq(p[1], "lz4"))
-            {
-                options->comp.alg = COMP_ALG_LZ4;
-                options->comp.flags |= COMP_F_SWAP;
-            }
-            else if (streq(p[1], "lz4-v2"))
-            {
-                options->comp.alg = COMP_ALGV2_LZ4;
-            }
-#endif
-            else
-            {
-                msg(msglevel, "bad comp option: %s", p[1]);
-                goto err;
-            }
+        else if (streq(alg, "lz4"))
+        {
+            options->comp.alg = COMP_ALG_LZ4;
+            options->comp.flags |= COMP_F_SWAP;
+        }
+        else if (streq(alg, "lz4-v2"))
+        {
+            options->comp.alg = COMP_ALGV2_LZ4;
         }
+#endif
         else
         {
-            options->comp.alg = COMP_ALG_STUB;
-            options->comp.flags |= COMP_F_SWAP;
+            msg(msglevel, "bad comp option: %s", alg);
+            goto err;
         }
+
         show_compression_warning(&options->comp);
     }
 #endif /* USE_COMP */