Skip to contentSkip to conversation

Remove count_netmask_bits(), convert users to use netmask_to_netbits2()

PATCH In review Gert Doering 2 participants 3 messages Dec 27, 2014 – Mar 8, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Remove count_netmask_bits(), convert users to use netmask_to_netbits2()

The previous Linux/iproute2 code converted binary netmasks to string representation (print_in_addr_t()), just to immediately scanf() it back to binary to count bits. netmask_to_netbits2() directly works on the in_addr_t.

---
 src/openvpn/misc.c  | 26 --------------------------
 src/openvpn/misc.h  |  4 ----
 src/openvpn/route.c |  4 ++--
 src/openvpn/route.h |  1 +
 src/openvpn/tun.c   |  4 ++--
 5 files changed, 5 insertions(+), 34 deletions(-)
src/openvpn/misc.c +0 -26
index 61bc523..5627cb9 100644
@@ -827,32 +827,6 @@ setenv_str_i (struct env_set *es, const char *name, const char *value, const int
   gc_free (&gc);
 }
 
-/*
- * taken from busybox networking/ifupdown.c
- */
-unsigned int
-count_bits(unsigned int a)
-{
-  unsigned int result;
-  result = (a & 0x55) + ((a >> 1) & 0x55);
-  result = (result & 0x33) + ((result >> 2) & 0x33);
-  return((result & 0x0F) + ((result >> 4) & 0x0F));
-}
-
-int
-count_netmask_bits(const char *dotted_quad)
-{
-  unsigned int result, a, b, c, d;
-  /* Found a netmask...  Check if it is dotted quad */
-  if (sscanf(dotted_quad, "%u.%u.%u.%u", &a, &b, &c, &d) != 4)
-    return -1;
-  result = count_bits(a);
-  result += count_bits(b);
-  result += count_bits(c);
-  result += count_bits(d);
-  return ((int)result);
-}
-
 /* return true if filename can be opened for read */
 bool
 test_file (const char *filename)
src/openvpn/misc.h +0 -4
index 41748bd..5fe085e 100644
@@ -166,10 +166,6 @@ const char **make_env_array (const struct env_set *es,
 const char **make_arg_array (const char *first, const char *parms, struct gc_arena *gc);
 const char **make_extended_arg_array (char **p, struct gc_arena *gc);
 
-/* convert netmasks for iproute2 */
-int count_netmask_bits(const char *);
-unsigned int count_bits(unsigned int );
-
 /* an analogue to the random() function, but use OpenSSL functions if available */
 #ifdef ENABLE_CRYPTO
 long int get_random(void);
src/openvpn/route.c +2 -2
index 5d18213..b56f620 100644
@@ -1288,7 +1288,7 @@ add_route (struct route_ipv4 *r,
   argv_printf (&argv, "%s route add %s/%d",
   	      iproute_path,
 	      network,
-             count_netmask_bits(netmask));
+              netmask_to_netbits2(r->netmask));
 
   if (r->flags & RT_METRIC_DEFINED)
     argv_printf_cat (&argv, "metric %d", r->metric);
@@ -1765,7 +1765,7 @@ delete_route (struct route_ipv4 *r,
   argv_printf (&argv, "%s route del %s/%d",
   	      iproute_path,
 	      network,
-	      count_netmask_bits(netmask));
+              netmask_to_netbits2(r->netmask));
 #else
   argv_printf (&argv, "%s del -net %s netmask %s",
 	       ROUTE_PATH,
src/openvpn/route.h +1 -0
index f3c0150..3cec08d 100644
@@ -305,6 +305,7 @@ static inline bool test_routes (const struct route_list *rl, const struct tuntap
 #endif
 
 bool netmask_to_netbits (const in_addr_t network, const in_addr_t netmask, int *netbits);
+int netmask_to_netbits2 (in_addr_t netmask);
 
 static inline in_addr_t
 netbits_to_netmask (const int netbits)
src/openvpn/tun.c +2 -2
index e92edc2..11a6d71 100644
@@ -722,7 +722,7 @@ do_ifconfig (struct tuntap *tt,
 				  iproute_path,
 				  actual,
 				  ifconfig_local,
-				  count_netmask_bits(ifconfig_remote_netmask),
+				  netmask_to_netbits2(tt->remote_netmask),
 				  ifconfig_broadcast
 				  );
 		  argv_msg (M_INFO, &argv);
@@ -1819,7 +1819,7 @@ close_tun (struct tuntap *tt)
 			iproute_path,
 			tt->actual_name,
 			print_in_addr_t (tt->local, 0, &gc),
-			count_netmask_bits(print_in_addr_t (tt->remote_netmask, 0, &gc))
+		        netmask_to_netbits2(tt->remote_netmask)
 			);
 	      }
 #else
Signature · 1 line
2.0.4
#1 lore raw

ACK

The new behaviour is slightly different, because it well reject invalid netmasks (e.g. 8.255.255.0). The old count_netmask_bits() would just happily report "there's 9 bits in that one!". I did not trace back the input of these calls to verify whether the netmasks can be invalid, but even if this somehow borks a setup, that setup must have been broken already...

-Steffan

Quoted 123 lines · Gert Doering

The previous Linux/iproute2 code converted binary netmasks to string representation (print_in_addr_t()), just to immediately scanf() it back to binary to count bits. netmask_to_netbits2() directly works on the in_addr_t.

---
  src/openvpn/misc.c  | 26 --------------------------
  src/openvpn/misc.h  |  4 ----
  src/openvpn/route.c |  4 ++--
  src/openvpn/route.h |  1 +
  src/openvpn/tun.c   |  4 ++--
  5 files changed, 5 insertions(+), 34 deletions(-)
src/openvpn/misc.c +0 -26
index 61bc523..5627cb9 100644
@@ -827,32 +827,6 @@ setenv_str_i (struct env_set *es, const char *name, const char *value, const int
    gc_free (&gc);
  }
 
-/*
- * taken from busybox networking/ifupdown.c
- */
-unsigned int
-count_bits(unsigned int a)
-{
-  unsigned int result;
-  result = (a & 0x55) + ((a >> 1) & 0x55);
-  result = (result & 0x33) + ((result >> 2) & 0x33);
-  return((result & 0x0F) + ((result >> 4) & 0x0F));
-}
-
-int
-count_netmask_bits(const char *dotted_quad)
-{
-  unsigned int result, a, b, c, d;
-  /* Found a netmask...  Check if it is dotted quad */
-  if (sscanf(dotted_quad, "%u.%u.%u.%u", &a, &b, &c, &d) != 4)
-    return -1;
-  result = count_bits(a);
-  result += count_bits(b);
-  result += count_bits(c);
-  result += count_bits(d);
-  return ((int)result);
-}
-
  /* return true if filename can be opened for read */
  bool
  test_file (const char *filename)
src/openvpn/misc.h +0 -4
index 41748bd..5fe085e 100644
@@ -166,10 +166,6 @@ const char **make_env_array (const struct env_set *es,
  const char **make_arg_array (const char *first, const char *parms, struct gc_arena *gc);
  const char **make_extended_arg_array (char **p, struct gc_arena *gc);
 
-/* convert netmasks for iproute2 */
-int count_netmask_bits(const char *);
-unsigned int count_bits(unsigned int );
-
  /* an analogue to the random() function, but use OpenSSL functions if available */
  #ifdef ENABLE_CRYPTO
  long int get_random(void);
src/openvpn/route.c +2 -2
index 5d18213..b56f620 100644
@@ -1288,7 +1288,7 @@ add_route (struct route_ipv4 *r,
    argv_printf (&argv, "%s route add %s/%d",
    	      iproute_path,
  	      network,
-             count_netmask_bits(netmask));
+              netmask_to_netbits2(r->netmask));
 
    if (r->flags & RT_METRIC_DEFINED)
      argv_printf_cat (&argv, "metric %d", r->metric);
@@ -1765,7 +1765,7 @@ delete_route (struct route_ipv4 *r,
    argv_printf (&argv, "%s route del %s/%d",
    	      iproute_path,
  	      network,
-	      count_netmask_bits(netmask));
+              netmask_to_netbits2(r->netmask));
  #else
    argv_printf (&argv, "%s del -net %s netmask %s",
  	       ROUTE_PATH,
src/openvpn/route.h +1 -0
index f3c0150..3cec08d 100644
@@ -305,6 +305,7 @@ static inline bool test_routes (const struct route_list *rl, const struct tuntap
  #endif
 
  bool netmask_to_netbits (const in_addr_t network, const in_addr_t netmask, int *netbits);
+int netmask_to_netbits2 (in_addr_t netmask);
 
  static inline in_addr_t
  netbits_to_netmask (const int netbits)
src/openvpn/tun.c +2 -2
index e92edc2..11a6d71 100644
@@ -722,7 +722,7 @@ do_ifconfig (struct tuntap *tt,
  				  iproute_path,
  				  actual,
  				  ifconfig_local,
-				  count_netmask_bits(ifconfig_remote_netmask),
+				  netmask_to_netbits2(tt->remote_netmask),
  				  ifconfig_broadcast
  				  );
  		  argv_msg (M_INFO, &argv);
@@ -1819,7 +1819,7 @@ close_tun (struct tuntap *tt)
  			iproute_path,
  			tt->actual_name,
  			print_in_addr_t (tt->local, 0, &gc),
-			count_netmask_bits(print_in_addr_t (tt->remote_netmask, 0, &gc))
+		        netmask_to_netbits2(tt->remote_netmask)
  			);
  	      }
  #else
#2 lore raw
In reply to Gert Doering

Your patch has been applied to the master branch.

commit ec2fbf374f018366c18644d271cd4d793d04244b

Author: Gert Doering
Date:   Sat Dec 27 21:59:43 2014 +0100
     Remove count_netmask_bits(), convert users to use netmask_to_netbits2()

     Signed-off-by: Gert Doering <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9360

--
kind regards,

Gert Doering