netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines
@ 2018-02-08 10:50 Serhey Popovych
  2018-02-08 10:50 ` [PATCH iproute2-next 1/3] vti/vti6: Unify vti_print_help() Serhey Popovych
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-02-08 10:50 UTC (permalink / raw)
  To: netdev

To show only relevant diffs of ip and ipv6 variants help message print
routines needs to be unified and improved.

Get rid of print_usage() and usage() wrappers: use single function to
output help message. As side effect we return -1 from parse function
instead of calling exit(2) in case of "... tunnel <help|garbage>" is
found.

Additionally we get pointer to @struct link_util and can directly access
->id information to prepare customized help message.

Split calls to fprintf() two group: one that contains format string with
specifiers (thus requiring parameters) and another one that does not.
This helps compiler to optimize calls to fprintf() with fputs() when no
format specifiers in string. Do not use fputs() directly to keep code
formatting nice.

After this series applied following diffs:

  # diff -urN ip/link_gre{,6}.c
  # diff -urN ip/link_vti{,6}.c
  # diff -urN ip/link_ip{,6}tnl.c

in scope of help print routines reduced to necessary minimum.

Tested minimally by compiling and executing "ip link help <kind>" and
"ip link add type help" commands. Looks correct.

See individual patch description for more information.

Reviews, commands and suggestions are welcome.

Thanks,
Serhii

Serhey Popovych (3):
  vti/vti6: Unify vti_print_help()
  gre/gre6: Unify gre_print_help()
  iptnl/ip6tnl: Unify iptunnel_print_help()

 ip/link_gre.c    |   73 ++++++++++++++++++++------------------------
 ip/link_gre6.c   |   74 +++++++++++++++++++++------------------------
 ip/link_ip6tnl.c |   58 ++++++++++++++++++-----------------
 ip/link_iptnl.c  |   88 ++++++++++++++++++++++++++----------------------------
 ip/link_vti.c    |   42 +++++++++++---------------
 ip/link_vti6.c   |   45 +++++++++++++---------------
 6 files changed, 178 insertions(+), 202 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next 1/3] vti/vti6: Unify vti_print_help()
  2018-02-08 10:50 [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines Serhey Popovych
@ 2018-02-08 10:50 ` Serhey Popovych
  2018-02-08 10:50 ` [PATCH iproute2-next 2/3] gre/gre6: Unify gre_print_help() Serhey Popovych
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-02-08 10:50 UTC (permalink / raw)
  To: netdev

Reduce diff lines between vti and vti6 help printing code.

Use @struct link_util ->id field to print correct link help:
all callers now pass this data structure to vti_print_help().

Get rid of custom print_usage() and usage() functions and use
vti_print_help() directly, return from function on "... type
<help|garbage>" instead of exit(2).

While there replace vti6 function prefix with vti to reduce
diff lines too.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_vti.c  |   42 ++++++++++++++++++------------------------
 ip/link_vti6.c |   45 ++++++++++++++++++++-------------------------
 2 files changed, 38 insertions(+), 49 deletions(-)

diff --git a/ip/link_vti.c b/ip/link_vti.c
index 49b87e9..f128e6b 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -23,29 +23,27 @@
 #include "ip_common.h"
 #include "tunnel.h"
 
-
-static void print_usage(FILE *f)
+static void vti_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
 {
 	fprintf(f,
-		"Usage: ... vti [ remote ADDR ]\n"
-		"               [ local ADDR ]\n"
-		"               [ [i|o]key KEY ]\n"
-		"               [ dev PHYS_DEV ]\n"
-		"               [ fwmark MARK ]\n"
+		"Usage: ... %-4s [ remote ADDR ]\n",
+		lu->id
+	);
+	fprintf(f,
+		"                [ local ADDR ]\n"
+		"                [ [i|o]key KEY ]\n"
+		"                [ dev PHYS_DEV ]\n"
+		"                [ fwmark MARK ]\n"
 		"\n"
-		"Where: ADDR := { IP_ADDRESS }\n"
+	);
+	fprintf(f,
+		"Where: ADDR := { IP%s_ADDRESS }\n"
 		"       KEY  := { DOTTED_QUAD | NUMBER }\n"
-		"       MARK := { 0x0..0xffffffff }\n"
+		"       MARK := { 0x0..0xffffffff }\n",
+		""
 	);
 }
 
-static void usage(void) __attribute__((noreturn));
-static void usage(void)
-{
-	print_usage(stderr);
-	exit(-1);
-}
-
 static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
 			 struct nlmsghdr *n)
 {
@@ -147,8 +145,10 @@ get_failed:
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
 				invarg("invalid fwmark\n", *argv);
-		} else
-			usage();
+		} else {
+			vti_print_help(lu, argc, argv, stderr);
+			return -1;
+		}
 		argc--; argv++;
 	}
 
@@ -208,12 +208,6 @@ static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 }
 
-static void vti_print_help(struct link_util *lu, int argc, char **argv,
-	FILE *f)
-{
-	print_usage(f);
-}
-
 struct link_util vti_link_util = {
 	.id = "vti",
 	.maxattr = IFLA_VTI_MAX,
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index d1fbec5..0a888cd 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -24,30 +24,29 @@
 #include "ip_common.h"
 #include "tunnel.h"
 
-static void print_usage(FILE *f)
+static void vti_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
 {
 	fprintf(f,
-		"Usage: ... vti6 [ remote ADDR ]\n"
+		"Usage: ... %-4s [ remote ADDR ]\n",
+		lu->id
+	);
+	fprintf(f,
 		"                [ local ADDR ]\n"
 		"                [ [i|o]key KEY ]\n"
 		"                [ dev PHYS_DEV ]\n"
 		"                [ fwmark MARK ]\n"
 		"\n"
-		"Where: ADDR := { IPV6_ADDRESS }\n"
+	);
+	fprintf(f,
+		"Where: ADDR := { IP%s_ADDRESS }\n"
 		"       KEY  := { DOTTED_QUAD | NUMBER }\n"
-		"       MARK := { 0x0..0xffffffff }\n"
+		"       MARK := { 0x0..0xffffffff }\n",
+		"V6"
 	);
 }
 
-static void usage(void) __attribute__((noreturn));
-static void usage(void)
-{
-	print_usage(stderr);
-	exit(-1);
-}
-
-static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
-			  struct nlmsghdr *n)
+static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
+			 struct nlmsghdr *n)
 {
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
@@ -153,8 +152,10 @@ get_failed:
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
 				invarg("invalid fwmark\n", *argv);
-		} else
-			usage();
+		} else {
+			vti_print_help(lu, argc, argv, stderr);
+			return -1;
+		}
 		argc--; argv++;
 	}
 
@@ -169,7 +170,7 @@ get_failed:
 	return 0;
 }
 
-static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+static void vti_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
 	char s2[64];
 
@@ -214,16 +215,10 @@ static void vti6_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	}
 }
 
-static void vti6_print_help(struct link_util *lu, int argc, char **argv,
-	FILE *f)
-{
-	print_usage(f);
-}
-
 struct link_util vti6_link_util = {
 	.id = "vti6",
 	.maxattr = IFLA_VTI_MAX,
-	.parse_opt = vti6_parse_opt,
-	.print_opt = vti6_print_opt,
-	.print_help = vti6_print_help,
+	.parse_opt = vti_parse_opt,
+	.print_opt = vti_print_opt,
+	.print_help = vti_print_help,
 };
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next 2/3] gre/gre6: Unify gre_print_help()
  2018-02-08 10:50 [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines Serhey Popovych
  2018-02-08 10:50 ` [PATCH iproute2-next 1/3] vti/vti6: Unify vti_print_help() Serhey Popovych
@ 2018-02-08 10:50 ` Serhey Popovych
  2018-02-08 10:50 ` [PATCH iproute2-next 3/3] iptnl/ip6tnl: Unify iptunnel_print_help() Serhey Popovych
  2018-02-09  3:35 ` [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines David Ahern
  3 siblings, 0 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-02-08 10:50 UTC (permalink / raw)
  To: netdev

Reduce diff lines between gre and gre6 help printing code.

Use @struct link_util ->id field to print correct link help:
all callers now pass this data structure to gre_print_help().

Get rid of custom print_usage() and usage() functions and use
gre_print_help() directly, return from function on "... type
<help|garbage>" instead of exit(2).

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c  |   73 +++++++++++++++++++++++++------------------------------
 ip/link_gre6.c |   74 ++++++++++++++++++++++++++------------------------------
 2 files changed, 67 insertions(+), 80 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index b2573a1..e972a10 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -23,34 +23,38 @@
 #include "ip_common.h"
 #include "tunnel.h"
 
-static void print_usage(FILE *f)
+static void gre_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
 {
 	fprintf(f,
-		"Usage: ... { gre | gretap | erspan } [ remote ADDR ]\n"
-		"                            [ local ADDR ]\n"
-		"                            [ [i|o]seq ]\n"
-		"                            [ [i|o]key KEY ]\n"
-		"                            [ [i|o]csum ]\n"
-		"                            [ ttl TTL ]\n"
-		"                            [ tos TOS ]\n"
-		"                            [ [no]pmtudisc ]\n"
-		"                            [ [no]ignore-df ]\n"
-		"                            [ dev PHYS_DEV ]\n"
-		"                            [ noencap ]\n"
-		"                            [ encap { fou | gue | none } ]\n"
-		"                            [ encap-sport PORT ]\n"
-		"                            [ encap-dport PORT ]\n"
-		"                            [ [no]encap-csum ]\n"
-		"                            [ [no]encap-csum6 ]\n"
-		"                            [ [no]encap-remcsum ]\n"
-		"                            [ external ]\n"
-		"                            [ fwmark MARK ]\n"
-		"                            [ erspan_ver version ]\n"
-		"                            [ erspan IDX ]\n"
-		"                            [ erspan_dir { ingress | egress } ]\n"
-		"                            [ erspan_hwid hwid ]\n"
-		"                            [ external ]\n"
+		"Usage: ... %-9s [ remote ADDR ]\n",
+		lu->id
+	);
+	fprintf(f,
+		"                     [ local ADDR ]\n"
+		"                     [ [i|o]seq ]\n"
+		"                     [ [i|o]key KEY ]\n"
+		"                     [ [i|o]csum ]\n"
+		"                     [ ttl TTL ]\n"
+		"                     [ tos TOS ]\n"
+		"                     [ [no]pmtudisc ]\n"
+		"                     [ [no]ignore-df ]\n"
+		"                     [ dev PHYS_DEV ]\n"
+		"                     [ fwmark MARK ]\n"
+		"                     [ external ]\n"
+		"                     [ noencap ]\n"
+		"                     [ encap { fou | gue | none } ]\n"
+		"                     [ encap-sport PORT ]\n"
+		"                     [ encap-dport PORT ]\n"
+		"                     [ [no]encap-csum ]\n"
+		"                     [ [no]encap-csum6 ]\n"
+		"                     [ [no]encap-remcsum ]\n"
+		"                     [ erspan_ver version ]\n"
+		"                     [ erspan IDX ]\n"
+		"                     [ erspan_dir { ingress | egress } ]\n"
+		"                     [ erspan_hwid hwid ]\n"
 		"\n"
+	);
+	fprintf(f,
 		"Where: ADDR := { IP_ADDRESS | any }\n"
 		"       TOS  := { NUMBER | inherit }\n"
 		"       TTL  := { 1..255 | inherit }\n"
@@ -59,13 +63,6 @@ static void print_usage(FILE *f)
 	);
 }
 
-static void usage(void) __attribute__((noreturn));
-static void usage(void)
-{
-	print_usage(stderr);
-	exit(-1);
-}
-
 static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 			 struct nlmsghdr *n)
 {
@@ -336,8 +333,10 @@ get_failed:
 			NEXT_ARG();
 			if (get_u16(&erspan_hwid, *argv, 0))
 				invarg("invalid erspan hwid\n", *argv);
-		} else
-			usage();
+		} else {
+			gre_print_help(lu, argc, argv, stderr);
+			return -1;
+		}
 		argc--; argv++;
 	}
 
@@ -517,12 +516,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			IFLA_GRE_ENCAP_DPORT);
 }
 
-static void gre_print_help(struct link_util *lu, int argc, char **argv,
-			   FILE *f)
-{
-	print_usage(f);
-}
-
 struct link_util gre_link_util = {
 	.id = "gre",
 	.maxattr = IFLA_GRE_MAX,
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 880b75d..1354f88 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -30,34 +30,39 @@
 
 #define DEFAULT_TNL_HOP_LIMIT	(64)
 
-static void print_usage(FILE *f)
+static void gre_print_help(struct link_util *lu, int argc, char **argv, FILE *f)
 {
 	fprintf(f,
-		"Usage: ... { ip6gre | ip6gretap | ip6erspan} [ remote ADDR ]\n"
-		"                                  [ local ADDR ]\n"
-		"                                  [ [i|o]seq ]\n"
-		"                                  [ [i|o]key KEY ]\n"
-		"                                  [ [i|o]csum ]\n"
-		"                                  [ hoplimit TTL ]\n"
-		"                                  [ encaplimit ELIM ]\n"
-		"                                  [ tclass TCLASS ]\n"
-		"                                  [ flowlabel FLOWLABEL ]\n"
-		"                                  [ dscp inherit ]\n"
-		"                                  [ fwmark MARK ]\n"
-		"                                  [ dev PHYS_DEV ]\n"
-		"                                  [ noencap ]\n"
-		"                                  [ encap { fou | gue | none } ]\n"
-		"                                  [ encap-sport PORT ]\n"
-		"                                  [ encap-dport PORT ]\n"
-		"                                  [ [no]encap-csum ]\n"
-		"                                  [ [no]encap-csum6 ]\n"
-		"                                  [ [no]encap-remcsum ]\n"
-		"                                  [ erspan_ver version ]\n"
-		"                                  [ erspan IDX ]\n"
-		"                                  [ erspan_dir { ingress | egress } ]\n"
-		"                                  [ erspan_hwid hwid ]\n"
-		"                                  [ external ]\n"
+		"Usage: ... %-9s [ remote ADDR ]\n",
+		lu->id
+	);
+	fprintf(f,
+		"                     [ local ADDR ]\n"
+		"                     [ [i|o]seq ]\n"
+		"                     [ [i|o]key KEY ]\n"
+		"                     [ [i|o]csum ]\n"
+		"                     [ hoplimit TTL ]\n"
+		"                     [ encaplimit ELIM ]\n"
+		"                     [ tclass TCLASS ]\n"
+		"                     [ flowlabel FLOWLABEL ]\n"
+		"                     [ dscp inherit ]\n"
+		"                     [ dev PHYS_DEV ]\n"
+		"                     [ fwmark MARK ]\n"
+		"                     [ external ]\n"
+		"                     [ noencap ]\n"
+		"                     [ encap { fou | gue | none } ]\n"
+		"                     [ encap-sport PORT ]\n"
+		"                     [ encap-dport PORT ]\n"
+		"                     [ [no]encap-csum ]\n"
+		"                     [ [no]encap-csum6 ]\n"
+		"                     [ [no]encap-remcsum ]\n"
+		"                     [ erspan_ver version ]\n"
+		"                     [ erspan IDX ]\n"
+		"                     [ erspan_dir { ingress | egress } ]\n"
+		"                     [ erspan_hwid hwid ]\n"
 		"\n"
+	);
+	fprintf(f,
 		"Where: ADDR      := IPV6_ADDRESS\n"
 		"       TTL       := { 0..255 } (default=%d)\n"
 		"       KEY       := { DOTTED_QUAD | NUMBER }\n"
@@ -69,13 +74,6 @@ static void print_usage(FILE *f)
 	);
 }
 
-static void usage(void) __attribute__((noreturn));
-static void usage(void)
-{
-	print_usage(stderr);
-	exit(-1);
-}
-
 static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 			 struct nlmsghdr *n)
 {
@@ -384,8 +382,10 @@ get_failed:
 			NEXT_ARG();
 			if (get_u16(&erspan_hwid, *argv, 0))
 				invarg("invalid erspan hwid\n", *argv);
-		} else
-			usage();
+		} else {
+			gre_print_help(lu, argc, argv, stderr);
+			return -1;
+		}
 		argc--; argv++;
 	}
 
@@ -584,12 +584,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			IFLA_GRE_ENCAP_DPORT);
 }
 
-static void gre_print_help(struct link_util *lu, int argc, char **argv,
-			   FILE *f)
-{
-	print_usage(f);
-}
-
 struct link_util ip6gre_link_util = {
 	.id = "ip6gre",
 	.maxattr = IFLA_GRE_MAX,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next 3/3] iptnl/ip6tnl: Unify iptunnel_print_help()
  2018-02-08 10:50 [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines Serhey Popovych
  2018-02-08 10:50 ` [PATCH iproute2-next 1/3] vti/vti6: Unify vti_print_help() Serhey Popovych
  2018-02-08 10:50 ` [PATCH iproute2-next 2/3] gre/gre6: Unify gre_print_help() Serhey Popovych
@ 2018-02-08 10:50 ` Serhey Popovych
  2018-02-09  3:35 ` [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines David Ahern
  3 siblings, 0 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-02-08 10:50 UTC (permalink / raw)
  To: netdev

Reduce diff lines between iptnl and ip6tnl help printing code.

Use @struct link_util ->id field to print correct link help:
all callers now pass this data structure to iptunnel_print_help().

Get rid of custom print_usage() and usage() functions and use
iptunnel_print_help() directly, return from function on "... type
<help|garbage>" instead of exit(2).

While there replace ip6tunnel function prefix with iptunnel to reduce
diff lines too.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_ip6tnl.c |   58 ++++++++++++++++++-----------------
 ip/link_iptnl.c  |   88 ++++++++++++++++++++++++++----------------------------
 2 files changed, 73 insertions(+), 73 deletions(-)

diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 91d7d99..54545ad 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -29,20 +29,26 @@
 
 #define DEFAULT_TNL_HOP_LIMIT	(64)
 
-static void print_usage(FILE *f)
+static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
+				FILE *f)
 {
+	const char *mode;
+
+	fprintf(f,
+		"Usage: ... %-6s [ remote ADDR ]\n",
+		lu->id
+	);
 	fprintf(f,
-		"Usage: ... ip6tnl [ mode { ip6ip6 | ipip6 | any } ]\n"
-		"                  [ remote ADDR ]\n"
 		"                  [ local ADDR ]\n"
-		"                  [ dev PHYS_DEV ]\n"
 		"                  [ encaplimit ELIM ]\n"
 		"                  [ hoplimit HLIM ]\n"
 		"                  [ tclass TCLASS ]\n"
 		"                  [ flowlabel FLOWLABEL ]\n"
 		"                  [ dscp inherit ]\n"
-		"                  [ fwmark MARK ]\n"
 		"                  [ [no]allow-localremote ]\n"
+		"                  [ dev PHYS_DEV ]\n"
+		"                  [ fwmark MARK ]\n"
+		"                  [ external ]\n"
 		"                  [ noencap ]\n"
 		"                  [ encap { fou | gue | none } ]\n"
 		"                  [ encap-sport PORT ]\n"
@@ -50,8 +56,14 @@ static void print_usage(FILE *f)
 		"                  [ [no]encap-csum ]\n"
 		"                  [ [no]encap-csum6 ]\n"
 		"                  [ [no]encap-remcsum ]\n"
-		"                  [ external ]\n"
-		"\n"
+	);
+	mode = "{ ip6ip6 | ipip6 | any }";
+	fprintf(f,
+		"                  [ mode %s ]\n"
+		"\n",
+		mode
+	);
+	fprintf(f,
 		"Where: ADDR      := IPV6_ADDRESS\n"
 		"       ELIM      := { none | 0..255 }(default=%d)\n"
 		"       HLIM      := 0..255 (default=%d)\n"
@@ -62,15 +74,8 @@ static void print_usage(FILE *f)
 	);
 }
 
-static void usage(void) __attribute__((noreturn));
-static void usage(void)
-{
-	print_usage(stderr);
-	exit(-1);
-}
-
-static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
-			       struct nlmsghdr *n)
+static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
+			      struct nlmsghdr *n)
 {
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
@@ -304,8 +309,10 @@ get_failed:
 			encapflags &= ~TUNNEL_ENCAP_FLAG_REMCSUM;
 		} else if (strcmp(*argv, "external") == 0) {
 			metadata = 1;
-		} else
-			usage();
+		} else {
+			iptunnel_print_help(lu, argc, argv, stderr);
+			return -1;
+		}
 		argc--, argv++;
 	}
 
@@ -331,7 +338,8 @@ get_failed:
 	return 0;
 }
 
-static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+static void iptunnel_print_opt(struct link_util *lu, FILE *f,
+			       struct rtattr *tb[])
 {
 	char s2[64];
 	int flags = 0;
@@ -456,16 +464,10 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			IFLA_IPTUN_ENCAP_DPORT);
 }
 
-static void ip6tunnel_print_help(struct link_util *lu, int argc, char **argv,
-				 FILE *f)
-{
-	print_usage(f);
-}
-
 struct link_util ip6tnl_link_util = {
 	.id = "ip6tnl",
 	.maxattr = IFLA_IPTUN_MAX,
-	.parse_opt = ip6tunnel_parse_opt,
-	.print_opt = ip6tunnel_print_opt,
-	.print_help = ip6tunnel_print_help,
+	.parse_opt = iptunnel_parse_opt,
+	.print_opt = iptunnel_print_opt,
+	.print_help = iptunnel_print_help,
 };
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 3e653b7..84117ac 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -24,49 +24,51 @@
 #include "ip_common.h"
 #include "tunnel.h"
 
-static void print_usage(FILE *f, int sit)
+static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
+				FILE *f)
 {
-	const char *type = sit ? "sit " : "ipip";
+	const char *mode;
 
 	fprintf(f,
-		"Usage: ... %s [ remote ADDR ]\n"
-		"                [ local ADDR ]\n"
-		"                [ ttl TTL ]\n"
-		"                [ tos TOS ]\n"
-		"                [ [no]pmtudisc ]\n"
-		"                [ dev PHYS_DEV ]\n"
-		"                [ 6rd-prefix ADDR ]\n"
-		"                [ 6rd-relay_prefix ADDR ]\n"
-		"                [ 6rd-reset ]\n"
-		"                [ noencap ]\n"
-		"                [ encap { fou | gue | none } ]\n"
-		"                [ encap-sport PORT ]\n"
-		"                [ encap-dport PORT ]\n"
-		"                [ [no]encap-csum ]\n"
-		"                [ [no]encap-csum6 ]\n"
-		"                [ [no]encap-remcsum ]\n",
-		type
+		"Usage: ... %-6s [ remote ADDR ]\n",
+		lu->id
+	);
+	fprintf(f,
+		"                  [ local ADDR ]\n"
+		"                  [ ttl TTL ]\n"
+		"                  [ tos TOS ]\n"
+		"                  [ [no]pmtudisc ]\n"
+		"                  [ 6rd-prefix ADDR ]\n"
+		"                  [ 6rd-relay_prefix ADDR ]\n"
+		"                  [ 6rd-reset ]\n"
+		"                  [ dev PHYS_DEV ]\n"
+		"                  [ fwmark MARK ]\n"
+		"                  [ external ]\n"
+		"                  [ noencap ]\n"
+		"                  [ encap { fou | gue | none } ]\n"
+		"                  [ encap-sport PORT ]\n"
+		"                  [ encap-dport PORT ]\n"
+		"                  [ [no]encap-csum ]\n"
+		"                  [ [no]encap-csum6 ]\n"
+		"                  [ [no]encap-remcsum ]\n"
 	);
-	if (sit) {
-		fprintf(f, "          [ mode { ip6ip | ipip | mplsip | any } ]\n");
-		fprintf(f, "          [ isatap ]\n");
+	if (strcmp(lu->id, "sit") == 0) {
+		mode = "{ ip6ip | ipip | mplsip | any } ]\n"
+		"                  [ isatap";
 	} else {
-		fprintf(f, "          [ mode { ipip | mplsip | any } ]\n");
+		mode = "{ ipip | mplsip | any }";
 	}
-	fprintf(f, "                [ external ]\n");
-	fprintf(f, "                [ fwmark MARK ]\n");
-	fprintf(f, "\n");
-	fprintf(f, "Where: ADDR := { IP_ADDRESS | any }\n");
-	fprintf(f, "       TOS  := { NUMBER | inherit }\n");
-	fprintf(f, "       TTL  := { 1..255 | inherit }\n");
-	fprintf(f, "       MARK := { 0x0..0xffffffff }\n");
-}
-
-static void usage(int sit) __attribute__((noreturn));
-static void usage(int sit)
-{
-	print_usage(stderr, sit);
-	exit(-1);
+	fprintf(f,
+		"                  [ mode %s ]\n"
+		"\n",
+		mode
+	);
+	fprintf(f,
+		"Where: ADDR := { IP_ADDRESS | any }\n"
+		"       TOS  := { NUMBER | inherit }\n"
+		"       TTL  := { 1..255 | inherit }\n"
+		"       MARK := { 0x0..0xffffffff }\n"
+	);
 }
 
 static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
@@ -313,8 +315,10 @@ get_failed:
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
 				invarg("invalid fwmark\n", *argv);
-		} else
-			usage(strcmp(lu->id, "sit") == 0);
+		} else {
+			iptunnel_print_help(lu, argc, argv, stderr);
+			return -1;
+		}
 		argc--, argv++;
 	}
 
@@ -483,12 +487,6 @@ static void iptunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[
 			IFLA_IPTUN_ENCAP_DPORT);
 }
 
-static void iptunnel_print_help(struct link_util *lu, int argc, char **argv,
-	FILE *f)
-{
-	print_usage(f, strcmp(lu->id, "sit") == 0);
-}
-
 struct link_util ipip_link_util = {
 	.id = "ipip",
 	.maxattr = IFLA_IPTUN_MAX,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines
  2018-02-08 10:50 [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines Serhey Popovych
                   ` (2 preceding siblings ...)
  2018-02-08 10:50 ` [PATCH iproute2-next 3/3] iptnl/ip6tnl: Unify iptunnel_print_help() Serhey Popovych
@ 2018-02-09  3:35 ` David Ahern
  2018-02-09  3:52   ` David Ahern
  3 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2018-02-09  3:35 UTC (permalink / raw)
  To: Serhey Popovych, netdev

On 2/8/18 3:50 AM, Serhey Popovych wrote:
> To show only relevant diffs of ip and ipv6 variants help message print
> routines needs to be unified and improved.
> 
> Get rid of print_usage() and usage() wrappers: use single function to
> output help message. As side effect we return -1 from parse function
> instead of calling exit(2) in case of "... tunnel <help|garbage>" is
> found.
> 
> Additionally we get pointer to @struct link_util and can directly access
> ->id information to prepare customized help message.
> 
> Split calls to fprintf() two group: one that contains format string with
> specifiers (thus requiring parameters) and another one that does not.
> This helps compiler to optimize calls to fprintf() with fputs() when no
> format specifiers in string. Do not use fputs() directly to keep code
> formatting nice.
> 
> After this series applied following diffs:
> 
>   # diff -urN ip/link_gre{,6}.c
>   # diff -urN ip/link_vti{,6}.c
>   # diff -urN ip/link_ip{,6}tnl.c
> 
> in scope of help print routines reduced to necessary minimum.
> 
> Tested minimally by compiling and executing "ip link help <kind>" and
> "ip link add type help" commands. Looks correct.
> 
> See individual patch description for more information.

Series applied to iproute2-next

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines
  2018-02-09  3:35 ` [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines David Ahern
@ 2018-02-09  3:52   ` David Ahern
  2018-02-09  6:48     ` Serhey Popovych
  0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2018-02-09  3:52 UTC (permalink / raw)
  To: Serhey Popovych, netdev

On 2/8/18 8:35 PM, David Ahern wrote:
> On 2/8/18 3:50 AM, Serhey Popovych wrote:
>> To show only relevant diffs of ip and ipv6 variants help message print
>> routines needs to be unified and improved.
>>
>> Get rid of print_usage() and usage() wrappers: use single function to
>> output help message. As side effect we return -1 from parse function
>> instead of calling exit(2) in case of "... tunnel <help|garbage>" is
>> found.
>>
>> Additionally we get pointer to @struct link_util and can directly access
>> ->id information to prepare customized help message.
>>
>> Split calls to fprintf() two group: one that contains format string with
>> specifiers (thus requiring parameters) and another one that does not.
>> This helps compiler to optimize calls to fprintf() with fputs() when no
>> format specifiers in string. Do not use fputs() directly to keep code
>> formatting nice.
>>
>> After this series applied following diffs:
>>
>>   # diff -urN ip/link_gre{,6}.c
>>   # diff -urN ip/link_vti{,6}.c
>>   # diff -urN ip/link_ip{,6}tnl.c
>>
>> in scope of help print routines reduced to necessary minimum.
>>
>> Tested minimally by compiling and executing "ip link help <kind>" and
>> "ip link add type help" commands. Looks correct.
>>
>> See individual patch description for more information.
> 
> Series applied to iproute2-next
> 
> 


I take that back. Before pushing I noticed you dropped the '6' from the
name all of the ipv6 print_help functions. Why?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines
  2018-02-09  3:52   ` David Ahern
@ 2018-02-09  6:48     ` Serhey Popovych
  0 siblings, 0 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-02-09  6:48 UTC (permalink / raw)
  To: David Ahern, netdev


[-- Attachment #1.1: Type: text/plain, Size: 1797 bytes --]

David Ahern wrote:
> On 2/8/18 8:35 PM, David Ahern wrote:
>> On 2/8/18 3:50 AM, Serhey Popovych wrote:
>>> To show only relevant diffs of ip and ipv6 variants help message print
>>> routines needs to be unified and improved.
>>>
>>> Get rid of print_usage() and usage() wrappers: use single function to
>>> output help message. As side effect we return -1 from parse function
>>> instead of calling exit(2) in case of "... tunnel <help|garbage>" is
>>> found.
>>>
>>> Additionally we get pointer to @struct link_util and can directly access
>>> ->id information to prepare customized help message.
>>>
>>> Split calls to fprintf() two group: one that contains format string with
>>> specifiers (thus requiring parameters) and another one that does not.
>>> This helps compiler to optimize calls to fprintf() with fputs() when no
>>> format specifiers in string. Do not use fputs() directly to keep code
>>> formatting nice.
>>>
>>> After this series applied following diffs:
>>>
>>>   # diff -urN ip/link_gre{,6}.c
>>>   # diff -urN ip/link_vti{,6}.c
>>>   # diff -urN ip/link_ip{,6}tnl.c
>>>
>>> in scope of help print routines reduced to necessary minimum.
>>>
>>> Tested minimally by compiling and executing "ip link help <kind>" and
>>> "ip link add type help" commands. Looks correct.
>>>
>>> See individual patch description for more information.
>>
>> Series applied to iproute2-next
>>
>>
> 
> 
> I take that back. Before pushing I noticed you dropped the '6' from the
> name all of the ipv6 print_help functions. Why?
> 

You are right. I probably should not do that change in this series. Will
send v2. There is no '6' in name for link_gre6.c for example so I
decided to drop it from the rest to reduce number of diffs.

Sorry for this.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-02-09  6:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-08 10:50 [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines Serhey Popovych
2018-02-08 10:50 ` [PATCH iproute2-next 1/3] vti/vti6: Unify vti_print_help() Serhey Popovych
2018-02-08 10:50 ` [PATCH iproute2-next 2/3] gre/gre6: Unify gre_print_help() Serhey Popovych
2018-02-08 10:50 ` [PATCH iproute2-next 3/3] iptnl/ip6tnl: Unify iptunnel_print_help() Serhey Popovych
2018-02-09  3:35 ` [PATCH iproute2-next 0/3] ip/tunnel: Unify tunnel help message print routines David Ahern
2018-02-09  3:52   ` David Ahern
2018-02-09  6:48     ` Serhey Popovych

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).