Skip to contentSkip to conversation

tun: Fix compiler warnings

PATCH Superseded David Sommerseth 3 participants 8 messages Nov 11, 2016 – Nov 16, 2016

lore
Share & tools
lore

David Sommerseth

author
#0 lore raw

PATCH 1/1 tun: Fix compiler warnings

Fixes two compiler warnings idetified by using CFLAGS=-O2

1) ifconfig_ipv6_remote is only used on Solaris, move the declaration and assignment of this variable into the TARGET_SOLARIS block.

2) Linux have it's own open_tun() function and does not depend on open_tun_generic() at all. So do not exclude open_tun_generic() if TARGET_LINUX is defined.

Signed-off-by David Sommerseth <[email protected]...>
---
 src/openvpn/tun.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
src/openvpn/tun.c +10 -4
index a312d91..1a3a88a 100644
@@ -751,7 +751,6 @@ do_ifconfig (struct tuntap *tt,
       const char *ifconfig_remote_netmask = NULL;
       const char *ifconfig_broadcast = NULL;
       const char *ifconfig_ipv6_local = NULL;
-      const char *ifconfig_ipv6_remote = NULL;
       bool do_ipv6 = false;
       struct argv argv;
 

@@ -774,7 +773,6 @@ do_ifconfig (struct tuntap *tt,
       if (tt->did_ifconfig_ipv6_setup )
         {
 	  ifconfig_ipv6_local = print_in6_addr (tt->local_ipv6, 0, &gc);
-	  ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
 	  do_ipv6 = true;
 	}
 

@@ -917,6 +915,12 @@ do_ifconfig (struct tuntap *tt,
       management_android_control (management, "IFCONFIG", buf_bptr(&out));
 
 #elif defined(TARGET_SOLARIS)
+      const char *ifconfig_ipv6_remote = NULL;
+
+      if (tt->did_ifconfig_ipv6_setup )
+        {
+	  ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
+        }
 
       /* Solaris 2.6 (and 7?) cannot set all parameters in one go...
        * example:

@@ -1504,7 +1508,7 @@ read_tun_header (struct tuntap* tt, uint8_t *buf, int len)
 #endif
 
 
-#ifndef WIN32
+#if !(defined(WIN32) || defined(TARGET_LINUX))
 static void
 open_tun_generic (const char *dev, const char *dev_type, const char *dev_node,
 		  bool dynamic, struct tuntap *tt)

@@ -1606,7 +1610,9 @@ open_tun_generic (const char *dev, const char *dev_type, const char *dev_node,
       tt->actual_name = string_alloc (dynamic_opened ? dynamic_name : dev, NULL);
     }
 }
+#endif // !WIN32 && !TARGET_LINUX
 
+#if !defined(WIN32)
 static void
 close_tun_generic (struct tuntap *tt)
 {

@@ -1617,7 +1623,7 @@ close_tun_generic (struct tuntap *tt)
   clear_tuntap (tt);
 }
 
-#endif
+#endif // !WIN32
 
 #if defined (TARGET_ANDROID)
 void
Signature · 1 line
1.8.3.1
#1 lore raw

Hi,

src/openvpn/tun.c do_ifconfig (struct tuntap *tt, · line 923

Quoted 10 lines · David Sommerseth
src/openvpn/tun.c +6 -0
@@ -917,6 +915,12 @@ do_ifconfig (struct tuntap *tt,
       management_android_control (management, "IFCONFIG", buf_bptr(&out));
 
 #elif defined(TARGET_SOLARIS)
+      const char *ifconfig_ipv6_remote = NULL;
+
+      if (tt->did_ifconfig_ipv6_setup )
+        {
+	  ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
+        }

NAK :-) - if you clean it up, do it right. Just stuff this into the if clause where it is used, not introducing yet another if(), and make the variable local to that block.

      if ( do_ipv6 )
        {
          argv_printf (&argv, "%s %s inet6 unplumb",
                            IFCONFIG_PATH, actual );
          argv_msg (M_INFO, &argv);
          openvpn_execve_check (&argv, es, 0, NULL);

          if ( tt->type == DEV_TYPE_TUN )
           {
+	      const char *ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
              argv_printf (&argv,
                            "%s %s inet6 plumb %s/%d %s up",
                            IFCONFIG_PATH,
                            actual,
                            ifconfig_ipv6_local,
                            tt->netbits_ipv6,
                            ifconfig_ipv6_remote
                            );
            }

src/openvpn/tun.c open_tun_generic (const char *dev, const char *dev_type, const char *dev_node, · line 1613

Quoted 5 lines
src/openvpn/tun.c +1 -0
@@ -1606,7 +1610,9 @@ open_tun_generic (const char *dev, const char *dev_type, const char *dev_node,
       tt->actual_name = string_alloc (dynamic_opened ? dynamic_name : dev, NULL);
     }
 }
+#endif // !WIN32 && !TARGET_LINUX

And NAK again - we do not do // comments.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#2 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 7 lines · Gert Doering

Hi,

Quoted 4 lines · David Sommerseth
(quoted diff) +0 -0
@@ -917,6 +915,12 @@ do_ifconfig (struct tuntap *tt, 
 management_android_control (management, "IFCONFIG",
 buf_bptr(&out));
Quoted 9 lines
Quoted 5 lines
(quoted diff) +0 -0
 #elif defined(TARGET_SOLARIS) +      const char
 *ifconfig_ipv6_remote = NULL; + +      if

(tt->did_ifconfig_ipv6_setup ) + { +
ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
+ }

NAK :-) - if you clean it up, do it right. Just stuff this into the if clause where it is used, not introducing yet another if(), and make the variable local to that block.

Meh, this is just silly. This was actually fixed in my local git index, I had just forgotten to do 'git add' to add the last improvements instead of the hacky "yes, this seems to fix it"-change.

Quoted 6 lines
Quoted 4 lines
(quoted diff) +0 -0
@@ -1606,7 +1610,9 @@ open_tun_generic (const char *dev, const

char *dev_type, const char *dev_node, tt->actual_name = string_alloc (dynamic_opened ? dynamic_name : dev, NULL); } } +#endif // !WIN32 && !TARGET_LINUX

And NAK again - we do not do // comments.

Duh! I dunno what I was thinking about here ... lunch? dinner? Not coding style.

- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYKkXmAAoJEIbPlEyWcf3y7yMP/3IB9xJ3XLhfwyKqXKUIikKP TSjJmZ1vWZVTUKSqmckoBRRdxvwZjuwGM6ortf99HP0KLqmWuuncLYi0irznHJQD 8Ee0ANjjEkWruUNc+oWsO7Gs/phTpQEXwrL3N2uvDS+w8bpFpP/hTkEoTrHo0ck8 OeGdh4y5Ry3rEB10TQxD+PYBTy0W+ewOHxFU1bWdl+8/msFSyOphNYeOmRpKFzEe xrVNOLyTczwblANgHc4GbDz4PG9xKdZsKdEGQSTEh2xfU1u8HMNwm2qNACBqTZQx /mr8nmd+6A3Gcyh69sdcqnLoWYaEVk1XNInl/QyxL4VXudEK03X9xKJHuUsaMkgh 4/Qs6tYbBt1I0fLdtALVcrJfEChyforWNz6BbDjlWJtFv054gq/XRK9GmtMKd2Fg UxOBxksnW9m7R300twnyz29pSOJ0vS+WfL5GahMaTDH9ArfmThnREbAWT68t+geT gQ1YfJKcK1BmNZSO5jC8jSYx4FQucb3UAds4BH9ip+OUqLD4+YD1JuoHM5oICYW/ wV31+itZ2pejmB/RZxxczHqTeByGYS0vw2N0AeZ5lp704fYUdzlyYFvBlK8gwFDw +i5JKZnM/yvVAf00GC2nZk7P/7Xhfde5W9IU2Czl3KOlSQQda/bAUvRJZNrZ2+CM tq4iqOL/5yY1OrUC7gT+
=p479
-----END PGP SIGNATURE-----

author
#3 lore raw

PATCH v2 1/1 tun: Fix compiler warnings

In reply to David Sommerseth

Fixes two compiler warnings idetified by using CFLAGS=-O2

1) ifconfig_ipv6_remote is only used on Solaris, move the declaration and assignment of this variable into the TARGET_SOLARIS block.

2) Linux have it's own open_tun() function and does not depend on open_tun_generic() at all. So do not exclude open_tun_generic() if TARGET_LINUX is defined.

v2 - Move changes from 1) into the proper if() block
directly
- Fix up incorrect comment tags related to changes in 2)

Signed-off-by David Sommerseth <[email protected]...>
---
 src/openvpn/tun.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
src/openvpn/tun.c +7 -5
index 2a87e09..8c74c8f 100644
@@ -751,7 +751,6 @@ do_ifconfig (struct tuntap *tt,
       const char *ifconfig_remote_netmask = NULL;
       const char *ifconfig_broadcast = NULL;
       const char *ifconfig_ipv6_local = NULL;
-      const char *ifconfig_ipv6_remote = NULL;
       bool do_ipv6 = false;
       struct argv argv = argv_new ();
 

@@ -772,7 +771,6 @@ do_ifconfig (struct tuntap *tt,
       if (tt->did_ifconfig_ipv6_setup )
         {
 	  ifconfig_ipv6_local = print_in6_addr (tt->local_ipv6, 0, &gc);
-	  ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
 	  do_ipv6 = true;
 	}
 

@@ -915,7 +913,6 @@ do_ifconfig (struct tuntap *tt,
       management_android_control (management, "IFCONFIG", buf_bptr(&out));
 
 #elif defined(TARGET_SOLARIS)
-
       /* Solaris 2.6 (and 7?) cannot set all parameters in one go...
        * example:
        *    ifconfig tun2 10.2.0.2 10.2.0.1 mtu 1450 up

@@ -977,6 +974,9 @@ do_ifconfig (struct tuntap *tt,
 
 	  if ( tt->type == DEV_TYPE_TUN )
 	   {
+	      const char *ifconfig_ipv6_remote = NULL;
+
+	      ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);
 	      argv_printf (&argv,
 			    "%s %s inet6 plumb %s/%d %s up",
 			    IFCONFIG_PATH,

@@ -1502,7 +1502,7 @@ read_tun_header (struct tuntap* tt, uint8_t *buf, int len)
 #endif
 
 
-#ifndef WIN32
+#if !(defined(WIN32) || defined(TARGET_LINUX))
 static void
 open_tun_generic (const char *dev, const char *dev_type, const char *dev_node,
 		  bool dynamic, struct tuntap *tt)

@@ -1604,7 +1604,9 @@ open_tun_generic (const char *dev, const char *dev_type, const char *dev_node,
       tt->actual_name = string_alloc (dynamic_opened ? dynamic_name : dev, NULL);
     }
 }
+#endif /* !WIN32 && !TARGET_LINUX */
 
+#if !defined(WIN32)
 static void
 close_tun_generic (struct tuntap *tt)
 {

@@ -1615,7 +1617,7 @@ close_tun_generic (struct tuntap *tt)
   clear_tuntap (tt);
 }
 
-#endif
+#endif /* !WIN32 */
 
 #if defined (TARGET_ANDROID)
 void
Signature · 1 line
1.8.3.1
#4 lore raw

Hi,

Quoted 1 line · David Sommerseth

Fixes two compiler warnings idetified by using CFLAGS=-O2

Spelling...

Quoted 7 lines

1) ifconfig_ipv6_remote is only used on Solaris, move the declaration and assignment of this variable into the TARGET_SOLARIS block.

2) Linux have it's own open_tun() function and does not depend on open_tun_generic() at all. So do not exclude open_tun_generic() if TARGET_LINUX is defined.

Should that be an "So do not include... " or "So exclude..."?

src/openvpn/tun.c do_ifconfig (struct tuntap *tt, · line 918

Quoted 5 lines
src/openvpn/tun.c +0 -1
@@ -915,7 +913,6 @@ do_ifconfig (struct tuntap *tt,
       management_android_control (management, "IFCONFIG", buf_bptr(&out));
 
 #elif defined(TARGET_SOLARIS)
-

Whitespace change - intentional?

src/openvpn/tun.c do_ifconfig (struct tuntap *tt, · line 979

Quoted 7 lines
src/openvpn/tun.c +3 -0
@@ -977,6 +974,9 @@ do_ifconfig (struct tuntap *tt,
 
 	  if ( tt->type == DEV_TYPE_TUN )
 	   {
+	      const char *ifconfig_ipv6_remote = NULL;
+
+	      ifconfig_ipv6_remote = print_in6_addr (tt->remote_ipv6, 0, &gc);

Why have an extra initialization with NULL here, if all you want is "assing print_in6_addr"? Single line should be good (unless too long).

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#5 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 6 lines · Gert Doering

Hi,

Quoted 1 line · David Sommerseth

Fixes two compiler warnings idetified by using CFLAGS=-O2

Spelling...

Meh. Will fix that one.

Quoted 9 lines
Quoted 7 lines

1) ifconfig_ipv6_remote is only used on Solaris, move the declaration and assignment of this variable into the TARGET_SOLARIS block.

2) Linux have it's own open_tun() function and does not depend on open_tun_generic() at all. So do not exclude open_tun_generic() if TARGET_LINUX is defined.

Should that be an "So do not include... " or "So exclude..."?

yes :)

Quoted 7 lines

Review do_ifconfig (struct tuntap *tt, · line 916

Quoted 5 lines
(quoted diff) +0 -0
@@ -915,7 +913,6 @@ do_ifconfig (struct tuntap *tt, 
 management_android_control (management, "IFCONFIG",
 buf_bptr(&out));
 
 #elif defined(TARGET_SOLARIS) -

Whitespace change - intentional?

Yes, but we can skip it if it is a deal-breaker.

Quoted 9 lines
Quoted 5 lines
(quoted diff) +0 -0
@@ -977,6 +974,9 @@ do_ifconfig (struct tuntap *tt,

if ( tt->type == DEV_TYPE_TUN ) { + const char
*ifconfig_ipv6_remote = NULL; + + ifconfig_ipv6_remote =
print_in6_addr (tt->remote_ipv6, 0, &gc);

Why have an extra initialization with NULL here, if all you want is "assing print_in6_addr"? Single line should be good (unless too long).

Yes, the line would exceed 80 characters, so this looked more readable.

- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYLIowAAoJEIbPlEyWcf3yqh4P/0XfeJtFkhyv0Zy9hpMvWqXd UUvzy1eWi+YDpD0yj1gnsNXvGuuDSKdYJ8N7xihxJ+YbGQaHYq6h4CSzy9v8LXc1 tcdStM+73cGFOWeJujLsazJYmYJVxicbSkIU0wqTdjqI55Ac55Qzp5F5mns9gVoO 2bMVMmfrNUWTG/vRK77VU0/0lfuNAeXp0WdY0Xzz3ee+hWxFaVorVidVwmEqfiWk 87mVLC79jdPjYqlk8jMmaw2Cd32/sn5rje+9Lg49P8VghFOduxqx8oD7PQKzp9sA HIcR0KPAPlHWgQpd+TX+ljHiEW/7jVrRuJNaubXSjFuDB/Ru9xxjJSL/DWPA4ahr jaw8d/K1AdivYGhNGs3LqvFOsEJiZEK1Keo2JOPVSo7AoZDWUuYw1B8h3EcACPHr RDG4Cay+tW6vQGyhcx+Q723f/ZNDG4pFfi4sYkiQ/xhQ4LZz3qR/J3hy13GjIVe0 lvg7vEZCJipPw7dgqDnyD4xrdlNwovkzSPv5GL/hQnyc+k9iYZyG825hYrftmDL6 BlJDyn2uXUXISHYSoibfRuBLOGEXjY14opWB1P5ZNHJL4NfdfPZOE2eZJvh7oF5J NqA14XvRGl0qlYsJN/bBJAHsf+RSKD3pKEwcNjIkJlCJj1AvBD4Nd/TQ51ok1r1r +R5LyZIRg0UtQrmWpZXi
=Xo64
-----END PGP SIGNATURE-----

#6 lore raw

Hi,

Quoted 4 lines · David Sommerseth
Quoted 3 lines · David Sommerseth
Quoted 1 line

Fixes two compiler warnings idetified by using CFLAGS=-O2

Spelling...

Meh. Will fix that one.

[..]

As discussed on IRC - there's a few issues with the patch (some stylistic disagreement, and WIN32 vs. _WIN32), but it compiles and passes IPv6-relevant tests on OpenSolaris, and that is the most important goal.

So, ACK, with amendments to the style and WIN32 bits on the fly.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#7 lore raw
In reply to David Sommerseth

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Your patch has been applied to the master branch

commit 7756043c01dd0b23252682f085f3d9a1f4b057de

Author: David Sommerseth
Date:   Fri Nov 11 14:11:13 2016 +0100
     tun: Fix compiler warnings

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg13063.html

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYLJEcAAoJEIbPlEyWcf3yk68QAImmNF0vdp1TsW0MA6d42GLD r+qXZirKTBiyuDvUdLzWPSSV6M9w/Y73aGWzKuCAZJLsQr2K3X/R1eA+blvo5X/E VR97oKEgCFoJRBqvsP8BzhuskIFLJaemniZKB3oR6T76q31HFB93nn8U0XeuXWhm 5YGjbwUorL0mQTpX4nmhon55wNumFCH0Mdf+HArdmC8jBjw9yLoJO/NUU0O8Wym/ MpN9H4nGsPCDL1TLBTZHBokEey093ecKD2A4GofnfeQEpBEWfRDzV7gK+jRPMy7X CH4/YZUFybfrVuxjOdEqMcGBlFmJChkMNMPa15bbrULYiLydOMTKprG6p9qDwnp3 TxMwRYMc0q0ZHeDsFb5g3oIB2FbKaxB0izzAQzuvpaHkeAwB5/ZjMIJCCB6XQH87 PazsTLP1FLqvpXIr6c3W49NKwd3BempE/ZoZlnGfQFuiRG0xbj74pGux1r3+RgV0 29/1PINKfqwJOavPa1FsStOJZeLBSr/RqD5Jbb6SXaX0aIC8xKSk55Qx1IXxUKFi O9EdJMvcDDjZsUDXh6s8BxOI7gtUnzq8cuCawcDJt/hd2Lqn+qXjgXJaay6CneFo pQtyd35bzFpLroK/s7/CZpT0qNga240a2bDngH2HAEbTkwClB2AxtXWE2rsiDwKQ 2uYOjH/4ArAc3Gunwoo+
=sYTf
-----END PGP SIGNATURE-----