[Openvpn-devel] [M] Change in openvpn[master]: options: Clean up function setenv_foreign_option

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

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

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

to review the following change.


Change subject: options: Clean up function setenv_foreign_option
......................................................................

options: Clean up function setenv_foreign_option

- Remove unnecessary len > 0 check by reordering
  the code slightly.
- Remove -Wconversion warning by making the len argument
  size_t.

Change-Id: I7efc77f63734501dfa8a8f5bed17b1a1b4e9e201
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/options.c
1 file changed, 25 insertions(+), 29 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/12/1112/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 70b5799..4d94211 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1067,41 +1067,37 @@
 
 #ifndef _WIN32
 static void
-setenv_foreign_option(struct options *o, const char *argv[], int len, struct env_set *es)
+setenv_foreign_option(struct options *o, const char *argv[], size_t len, struct env_set *es)
 {
-    if (len > 0)
-    {
-        struct gc_arena gc = gc_new();
-        struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
-        struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
-        int i;
-        bool first = true;
-        bool good = true;
+    struct gc_arena gc = gc_new();
+    struct buffer name = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+    struct buffer value = alloc_buf_gc(OPTION_PARM_SIZE, &gc);
+    bool first = true;
+    bool good = true;
 
-        good &= buf_printf(&name, "foreign_option_%d", o->foreign_option_index + 1);
-        ++o->foreign_option_index;
-        for (i = 0; i < len; ++i)
+    good &= buf_printf(&name, "foreign_option_%d", o->foreign_option_index + 1);
+    for (size_t i = 0; i < len; ++i)
+    {
+        if (argv[i])
         {
-            if (argv[i])
+            if (!first)
             {
-                if (!first)
-                {
-                    good &= buf_printf(&value, " ");
-                }
-                good &= buf_printf(&value, "%s", argv[i]);
-                first = false;
+                good &= buf_printf(&value, " ");
             }
+            good &= buf_printf(&value, "%s", argv[i]);
+            first = false;
         }
-        if (good)
-        {
-            setenv_str(es, BSTR(&name), BSTR(&value));
-        }
-        else
-        {
-            msg(M_WARN, "foreign_option: name/value overflow");
-        }
-        gc_free(&gc);
     }
+    if (good)
+    {
+        setenv_str(es, BSTR(&name), BSTR(&value));
+        ++o->foreign_option_index;
+    }
+    else
+    {
+        msg(M_WARN, "foreign_option: name/value overflow");
+    }
+    gc_free(&gc);
 }
 #endif /* ifndef _WIN32 */
 
@@ -3679,7 +3675,7 @@
     {
         /* Set foreign option env vars from --dns config */
         const char *p[] = { "dhcp-option", NULL, NULL };
-        size_t p_len = sizeof(p) / sizeof(p[0]);
+        const size_t p_len = sizeof(p) / sizeof(p[0]);
 
         p[1] = "DOMAIN";
         const struct dns_domain *d = dns->search_domains;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1112?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: I7efc77f63734501dfa8a8f5bed17b1a1b4e9e201
Gerrit-Change-Number: 1112
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange