Guided option parser

Guided option parser

[Jan Engelhardt]

This is a staggered submit, with 10 patches in the first submission.
See 01/10 for a description.

Currently, the entire set of 64 has a diffstat of

 74 files changed, 2845 insertions(+), 4062 deletions(-)

this is roughly a third of the entire work, and it is expected this
to linearly continue until ~200 patches. But I felt I needed to flush
the queue now.

===
The following changes since commit a239728ec064666025de2723997d87b176d57fd6:

  mark newly opened fds as FD_CLOEXEC (close on exec) (2011-04-04 15:30:32 +0200)

are available in the git repository at:
  git://dev.medozas.de/iptables opts

Jan Engelhardt (10):
      extensions: add missing checks for specific flags (2)
      libxtables: guided option parser
      libxt_CHECKSUM: use guided option parser
      libxt_socket: use guided option parser
      libxtables: provide better final_check
      libxt_CONNSECMARK: use guided option parser
      libxtables: XTTYPE_UINT32 support
      libxt_cpu: use guided option parser
      libxtables: min-max option support
      libxt_cluster: use guided option parser

 Makefile.am                     |    2 +-
 extensions/libip6t_ipv6header.c |    4 +-
 extensions/libxt_CHECKSUM.c     |   46 ++----
 extensions/libxt_CONNSECMARK.c  |   59 +++----
 extensions/libxt_cluster.c      |  187 +++++---------------
 extensions/libxt_cpu.c          |   65 ++-----
 extensions/libxt_socket.c       |   29 ++--
 include/xtables.h.in            |  111 ++++++++++++
 ip6tables.c                     |   56 ++++--
 iptables.c                      |   58 ++++---
 xshared.h                       |   20 ++
 xtables.c                       |   25 +--
 xtoptions.c                     |  366 +++++++++++++++++++++++++++++++++++++++
 13 files changed, 695 insertions(+), 333 deletions(-)
 create mode 100644 xtoptions.c

[PATCH 01/10] extensions: add missing checks for specific flags (2)

[Jan Engelhardt]

Addendum to v1.4.10-75-g4e5d4bf. It does not make sense to use
ipv6header's --soft without specifying any options.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libip6t_ipv6header.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/extensions/libip6t_ipv6header.c b/extensions/libip6t_ipv6header.c
index 187e0a4..c05cedb 100644
--- a/extensions/libip6t_ipv6header.c
+++ b/extensions/libip6t_ipv6header.c
@@ -202,7 +202,9 @@ ipv6header_parse(int c, char **argv, int invert, unsigned int *flags,
 
 static void ipv6header_check(unsigned int flags)
 {
-	if (!flags) xtables_error(PARAMETER_PROBLEM, "ip6t_ipv6header: no options specified");
+	if (!(flags & IPV6_HDR_HEADER))
+		xtables_error(PARAMETER_PROBLEM,
+			"ip6t_ipv6header: no options specified");
 }
 
 static void
-- 
1.7.1

[PATCH 02/10] libxtables: guided option parser

[Jan Engelhardt]

This patchset seeks to drastically reduce the code in the individual
extensions by centralizing their argument parsing (breakdown of
strings), validation, and in part, assignment.

As a secondary goal, this reduces the number of static storage duration
variables in flight.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 Makefile.am          |    2 +-
 include/xtables.h.in |   91 +++++++++++++++
 ip6tables.c          |   58 +++++++---
 iptables.c           |   60 +++++++---
 xshared.h            |   20 ++++
 xtables.c            |   25 +---
 xtoptions.c          |  299 ++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 501 insertions(+), 54 deletions(-)
 create mode 100644 xtoptions.c

diff --git a/Makefile.am b/Makefile.am
index 7f0eb2f..fbed41f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -27,7 +27,7 @@ libiptc_libip6tc_la_SOURCES = libiptc/libip6tc.c
 libiptc_libip6tc_la_LDFLAGS = -version-info 0:0:0 ${libiptc_LDFLAGS2}
 
 lib_LTLIBRARIES      += libxtables.la
-libxtables_la_SOURCES = xtables.c
+libxtables_la_SOURCES = xtables.c xtoptions.c
 libxtables_la_LDFLAGS = -version-info ${libxtables_vcurrent}:0:${libxtables_vage}
 if ENABLE_SHARED
 libxtables_la_CFLAGS  = ${AM_CFLAGS}
diff --git a/include/xtables.h.in b/include/xtables.h.in
index c3d34af..928f465 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -10,6 +10,8 @@
 #include <sys/types.h>
 #include <limits.h>
 #include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
 #include <netinet/in.h>
 #include <net/if.h>
 #include <linux/types.h>
@@ -34,6 +36,73 @@
 
 struct in_addr;
 
+/*
+ * .size is here so that there is a somewhat reasonable check
+ * against the chosen .type.
+ */
+#define XTOPT_POINTER(stype, member) \
+	.ptroff = offsetof(stype, member), \
+	.size = sizeof(((stype *)NULL)->member)
+#define XTOPT_TABLEEND {.name = NULL}
+
+/**
+ * %XTTYPE_NONE:	option takes no argument
+ */
+enum xt_option_type {
+	XTTYPE_NONE,
+};
+
+/**
+ * %XTOPT_INVERT:	option is invertible (usable with !)
+ * %XTOPT_MAND:		option is mandatory
+ * %XTOPT_MULTI:	option may be specified multiple times
+ * %XTOPT_PUT:		store value into memory at @ptroff
+ */
+enum xt_option_flags {
+	XTOPT_INVERT = 1 << 0,
+	XTOPT_MAND   = 1 << 1,
+	XTOPT_MULTI  = 1 << 2,
+	XTOPT_PUT    = 1 << 3,
+};
+
+/**
+ * @name:	name of option
+ * @type:	type of input and validation method, see %XTTYPE_*
+ * @id:		unique number (within extension) for option, 0-31
+ * @excl:	bitmask of flags that cannot be used with this option
+ * @also:	bitmask of flags that must be used with this option
+ * @flags:	bitmask of option flags, see %XTOPT_*
+ * @ptroff:	offset into private structure for member
+ * @size:	size of the item pointed to by @ptroff; this is a safeguard
+ */
+struct xt_option_entry {
+	const char *name;
+	enum xt_option_type type;
+	unsigned int id, excl, also, flags;
+	unsigned int ptroff;
+	size_t size;
+};
+
+/**
+ * @arg:	input from command line
+ * @ext_name:	name of extension currently being processed
+ * @entry:	current option being processed
+ * @data:	per-extension data block
+ * @xflags:	options of the extension that have been used
+ * @invert:	whether option was used with !
+ * @val:	parsed result
+ */
+struct xt_option_call {
+	const char *arg, *ext_name;
+	const struct xt_option_entry *entry;
+	void *data;
+	unsigned int xflags;
+	bool invert;
+	union {
+		/* to be filled */
+	} val;
+};
+
 /* Include file for additions: new matches and targets. */
 struct xtables_match
 {
@@ -86,6 +155,10 @@ struct xtables_match
 	/* Pointer to list of extra command-line options */
 	const struct option *extra_opts;
 
+	/* New parser */
+	void (*x6_parse)(struct xt_option_call *);
+	const struct xt_option_entry *x6_options;
+
 	/* Ignore these men behind the curtain: */
 	unsigned int option_offset;
 	struct xt_entry_match *m;
@@ -145,6 +218,10 @@ struct xtables_target
 	/* Pointer to list of extra command-line options */
 	const struct option *extra_opts;
 
+	/* New parser */
+	void (*x6_parse)(struct xt_option_call *);
+	const struct xt_option_entry *x6_options;
+
 	/* Ignore these men behind the curtain: */
 	unsigned int option_offset;
 	struct xt_entry_target *t;
@@ -292,6 +369,20 @@ extern void xtables_save_string(const char *value);
 extern const struct xtables_pprot xtables_chain_protos[];
 extern u_int16_t xtables_parse_protocol(const char *s);
 
+/* xtoptions.c */
+extern void xtables_option_metavalidate(const char *,
+					const struct xt_option_entry *);
+extern struct option *xtables_options_xfrm(struct option *, struct option *,
+					   const struct xt_option_entry *,
+					   unsigned int *);
+extern void xtables_option_parse(struct xt_option_call *);
+extern void xtables_option_tpcall(unsigned int, char **, bool,
+				  struct xtables_target *, void *);
+extern void xtables_option_mpcall(unsigned int, char **, bool,
+				  struct xtables_match *, void *);
+extern void xtables_options_fcheck(const char *, unsigned int,
+				   const struct xt_option_entry *);
+
 #ifdef XTABLES_INTERNAL
 
 /* Shipped modules rely on this... */
diff --git a/ip6tables.c b/ip6tables.c
index 96a0fdc..83d2fae 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -1279,25 +1279,25 @@ static void command_default(struct iptables_command_state *cs)
 	struct xtables_rule_match *matchp;
 	struct xtables_match *m;
 
-	if (cs->target != NULL && cs->target->parse != NULL &&
+	if (cs->target != NULL &&
+	    (cs->target->parse != NULL || cs->target->x6_parse != NULL) &&
 	    cs->c >= cs->target->option_offset &&
 	    cs->c < cs->target->option_offset + XT_OPTION_OFFSET_SCALE) {
-		cs->target->parse(cs->c - cs->target->option_offset, cs->argv,
-				  cs->invert, &cs->target->tflags, &cs->fw6,
-				  &cs->target->t);
+		xtables_option_tpcall(cs->c, cs->argv, cs->invert,
+				      cs->target, &cs->fw);
 		return;
 	}
 
 	for (matchp = cs->matches; matchp; matchp = matchp->next) {
 		m = matchp->match;
 
-		if (matchp->completed || m->parse == NULL)
+		if (matchp->completed ||
+		    (m->x6_parse == NULL && m->parse == NULL))
 			continue;
 		if (cs->c < matchp->match->option_offset ||
 		    cs->c >= matchp->match->option_offset + XT_OPTION_OFFSET_SCALE)
 			continue;
-		m->parse(cs->c - m->option_offset, cs->argv, cs->invert,
-			 &m->mflags, &cs->fw6, &m->m);
+		xtables_option_mpcall(cs->c, cs->argv, cs->invert, m, &cs->fw);
 		return;
 	}
 
@@ -1317,9 +1317,17 @@ static void command_default(struct iptables_command_state *cs)
 		if (m->init != NULL)
 			m->init(m->m);
 
-		opts = xtables_merge_options(ip6tables_globals.orig_opts, opts,
-					     m->extra_opts, &m->option_offset);
-
+		if (m->x6_options != NULL)
+			opts = xtables_options_xfrm(ip6tables_globals.orig_opts,
+						    opts, m->x6_options,
+						    &m->option_offset);
+		else
+			opts = xtables_merge_options(ip6tables_globals.orig_opts,
+						     opts,
+						     m->extra_opts,
+						     &m->option_offset);
+		if (opts == NULL)
+			xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 		optind--;
 		return;
 	}
@@ -1353,9 +1361,14 @@ static void command_jump(struct iptables_command_state *cs)
 	cs->target->t->u.user.revision = cs->target->revision;
 	if (cs->target->init != NULL)
 		cs->target->init(cs->target->t);
-	opts = xtables_merge_options(ip6tables_globals.orig_opts, opts,
-				     cs->target->extra_opts,
-				     &cs->target->option_offset);
+	if (cs->target->x6_options != NULL)
+		opts = xtables_options_xfrm(ip6tables_globals.orig_opts, opts,
+					    cs->target->x6_options,
+					    &cs->target->option_offset);
+	else
+		opts = xtables_merge_options(ip6tables_globals.orig_opts, opts,
+					     cs->target->extra_opts,
+					     &cs->target->option_offset);
 	if (opts == NULL)
 		xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 }
@@ -1377,8 +1390,13 @@ static void command_match(struct iptables_command_state *cs)
 	m->m->u.user.revision = m->revision;
 	if (m->init != NULL)
 		m->init(m->m);
-	if (m != m->next)
-		/* Merge options for non-cloned matches */
+	if (m == m->next)
+		return;
+	/* Merge options for non-cloned matches */
+	if (m->x6_options != NULL)
+		opts = xtables_options_xfrm(ip6tables_globals.orig_opts, opts,
+					    m->x6_options, &m->option_offset);
+	else if (m->extra_opts != NULL)
 		opts = xtables_merge_options(ip6tables_globals.orig_opts, opts,
 					     m->extra_opts, &m->option_offset);
 }
@@ -1764,10 +1782,18 @@ int do_command6(int argc, char *argv[], char **table, struct ip6tc_handle **hand
 		cs.invert = FALSE;
 	}
 
-	for (matchp = cs.matches; matchp; matchp = matchp->next)
+	for (matchp = cs.matches; matchp; matchp = matchp->next) {
+		if (matchp->match->x6_options != NULL)
+			xtables_options_fcheck(matchp->match->name,
+					       matchp->match->mflags,
+					       matchp->match->x6_options);
 		if (matchp->match->final_check != NULL)
 			matchp->match->final_check(matchp->match->mflags);
+	}
 
+	if (cs.target != NULL && cs.target->x6_options != NULL)
+		xtables_options_fcheck(cs.target->name, cs.target->tflags,
+				       cs.target->x6_options);
 	if (cs.target != NULL && cs.target->final_check != NULL)
 		cs.target->final_check(cs.target->tflags);
 
diff --git a/iptables.c b/iptables.c
index cff4a7b..269a66f 100644
--- a/iptables.c
+++ b/iptables.c
@@ -1303,25 +1303,25 @@ static void command_default(struct iptables_command_state *cs)
 	struct xtables_rule_match *matchp;
 	struct xtables_match *m;
 
-	if (cs->target != NULL && cs->target->parse != NULL &&
+	if (cs->target != NULL &&
+	    (cs->target->parse != NULL || cs->target->x6_parse != NULL) &&
 	    cs->c >= cs->target->option_offset &&
 	    cs->c < cs->target->option_offset + XT_OPTION_OFFSET_SCALE) {
-		cs->target->parse(cs->c - cs->target->option_offset, cs->argv,
-				  cs->invert, &cs->target->tflags, &cs->fw,
-				  &cs->target->t);
+		xtables_option_tpcall(cs->c, cs->argv, cs->invert,
+				      cs->target, &cs->fw);
 		return;
 	}
 
 	for (matchp = cs->matches; matchp; matchp = matchp->next) {
 		m = matchp->match;
 
-		if (matchp->completed || m->parse == NULL)
+		if (matchp->completed ||
+		    (m->x6_parse == NULL && m->parse == NULL))
 			continue;
 		if (cs->c < m->option_offset ||
 		    cs->c >= m->option_offset + XT_OPTION_OFFSET_SCALE)
 			continue;
-		m->parse(cs->c - m->option_offset, cs->argv, cs->invert,
-			 &m->mflags, &cs->fw, &m->m);
+		xtables_option_mpcall(cs->c, cs->argv, cs->invert, m, &cs->fw);
 		return;
 	}
 
@@ -1341,8 +1341,15 @@ static void command_default(struct iptables_command_state *cs)
 		if (m->init != NULL)
 			m->init(m->m);
 
-		opts = xtables_merge_options(iptables_globals.orig_opts, opts,
-					     m->extra_opts, &m->option_offset);
+		if (m->x6_options != NULL)
+			opts = xtables_options_xfrm(iptables_globals.orig_opts,
+						    opts, m->x6_options,
+						    &m->option_offset);
+		else
+			opts = xtables_merge_options(iptables_globals.orig_opts,
+						     opts,
+						     m->extra_opts,
+						     &m->option_offset);
 		if (opts == NULL)
 			xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 
@@ -1380,9 +1387,14 @@ static void command_jump(struct iptables_command_state *cs)
 	cs->target->t->u.user.revision = cs->target->revision;
 	if (cs->target->init != NULL)
 		cs->target->init(cs->target->t);
-	opts = xtables_merge_options(iptables_globals.orig_opts, opts,
-				     cs->target->extra_opts,
-				     &cs->target->option_offset);
+	if (cs->target->x6_options != NULL)
+		opts = xtables_options_xfrm(iptables_globals.orig_opts, opts,
+					    cs->target->x6_options,
+					    &cs->target->option_offset);
+	else
+		opts = xtables_merge_options(iptables_globals.orig_opts, opts,
+					     cs->target->extra_opts,
+					     &cs->target->option_offset);
 	if (opts == NULL)
 		xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 }
@@ -1404,13 +1416,17 @@ static void command_match(struct iptables_command_state *cs)
 	m->m->u.user.revision = m->revision;
 	if (m->init != NULL)
 		m->init(m->m);
-	if (m != m->next) {
-		/* Merge options for non-cloned matches */
+	if (m == m->next)
+		return;
+	/* Merge options for non-cloned matches */
+	if (m->x6_options != NULL)
+		opts = xtables_options_xfrm(iptables_globals.orig_opts, opts,
+					    m->x6_options, &m->option_offset);
+	else if (m->extra_opts != NULL)
 		opts = xtables_merge_options(iptables_globals.orig_opts, opts,
 					     m->extra_opts, &m->option_offset);
-		if (opts == NULL)
-			xtables_error(OTHER_PROBLEM, "can't alloc memory!");
-	}
+	if (opts == NULL)
+		xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 }
 
 int do_command(int argc, char *argv[], char **table, struct iptc_handle **handle)
@@ -1800,10 +1816,18 @@ int do_command(int argc, char *argv[], char **table, struct iptc_handle **handle
 			"\nThe \"nat\" table is not intended for filtering, "
 		        "the use of DROP is therefore inhibited.\n\n");
 
-	for (matchp = cs.matches; matchp; matchp = matchp->next)
+	for (matchp = cs.matches; matchp; matchp = matchp->next) {
+		if (matchp->match->x6_options != NULL)
+			xtables_options_fcheck(matchp->match->name,
+					       matchp->match->mflags,
+					       matchp->match->x6_options);
 		if (matchp->match->final_check != NULL)
 			matchp->match->final_check(matchp->match->mflags);
+	}
 
+	if (cs.target != NULL && cs.target->x6_options != NULL)
+		xtables_options_fcheck(cs.target->name, cs.target->tflags,
+				       cs.target->x6_options);
 	if (cs.target != NULL && cs.target->final_check != NULL)
 		cs.target->final_check(cs.target->tflags);
 
diff --git a/xshared.h b/xshared.h
index 94abb39..be53535 100644
--- a/xshared.h
+++ b/xshared.h
@@ -26,6 +26,24 @@ enum {
 struct xtables_rule_match;
 struct xtables_target;
 
+/**
+ * xtables_afinfo - protocol family dependent information
+ * @kmod:		kernel module basename (e.g. "ip_tables")
+ * @libprefix:		prefix of .so library name (e.g. "libipt_")
+ * @family:		nfproto family
+ * @ipproto:		used by setsockopt (e.g. IPPROTO_IP)
+ * @so_rev_match:	optname to check revision support of match
+ * @so_rev_target:	optname to check revision support of target
+ */
+struct xtables_afinfo {
+	const char *kmod;
+	const char *libprefix;
+	uint8_t family;
+	uint8_t ipproto;
+	int so_rev_match;
+	int so_rev_target;
+};
+
 struct iptables_command_state {
 	union {
 		struct ipt_entry fw;
@@ -59,4 +77,6 @@ extern const char *proto_to_name(uint8_t, int);
 extern struct xtables_match *load_proto(struct iptables_command_state *);
 extern int subcmd_main(int, char **, const struct subcommand *);
 
+extern const struct xtables_afinfo *afinfo;
+
 #endif /* IPTABLES_XSHARED_H */
diff --git a/xtables.c b/xtables.c
index 352963f..235e2b2 100644
--- a/xtables.c
+++ b/xtables.c
@@ -49,6 +49,7 @@
 #	define IP6T_SO_GET_REVISION_TARGET	69
 #endif
 #include <getopt.h>
+#include "iptables/internal.h"
 #include "xshared.h"
 
 #define NPROTO	255
@@ -130,24 +131,6 @@ struct option *xtables_merge_options(struct option *orig_opts,
 	return merge;
 }
 
-/**
- * xtables_afinfo - protocol family dependent information
- * @kmod:		kernel module basename (e.g. "ip_tables")
- * @libprefix:		prefix of .so library name (e.g. "libipt_")
- * @family:		nfproto family
- * @ipproto:		used by setsockopt (e.g. IPPROTO_IP)
- * @so_rev_match:	optname to check revision support of match
- * @so_rev_target:	optname to check revision support of target
- */
-struct xtables_afinfo {
-	const char *kmod;
-	const char *libprefix;
-	uint8_t family;
-	uint8_t ipproto;
-	int so_rev_match;
-	int so_rev_target;
-};
-
 static const struct xtables_afinfo afinfo_ipv4 = {
 	.kmod          = "ip_tables",
 	.libprefix     = "libipt_",
@@ -166,7 +149,7 @@ static const struct xtables_afinfo afinfo_ipv6 = {
 	.so_rev_target = IP6T_SO_GET_REVISION_TARGET,
 };
 
-static const struct xtables_afinfo *afinfo;
+const struct xtables_afinfo *afinfo;
 
 /* Search path for Xtables .so files */
 static const char *xtables_libdir;
@@ -785,6 +768,8 @@ void xtables_register_match(struct xtables_match *me)
 		exit(1);
 	}
 
+	if (me->x6_options != NULL)
+		xtables_option_metavalidate(me->name, me->x6_options);
 	if (me->extra_opts != NULL)
 		xtables_check_options(me->name, me->extra_opts);
 
@@ -873,6 +858,8 @@ void xtables_register_target(struct xtables_target *me)
 		exit(1);
 	}
 
+	if (me->x6_options != NULL)
+		xtables_option_metavalidate(me->name, me->x6_options);
 	if (me->extra_opts != NULL)
 		xtables_check_options(me->name, me->extra_opts);
 
diff --git a/xtoptions.c b/xtoptions.c
new file mode 100644
index 0000000..3286aa1
--- /dev/null
+++ b/xtoptions.c
@@ -0,0 +1,299 @@
+/*
+ *	Argument parser
+ *	Copyright © Jan Engelhardt, 2011
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License as
+ *	published by the Free Software Foundation; either version 2 of
+ *	the License, or (at your option) any later version.
+ */
+#include <errno.h>
+#include <getopt.h>
+#include <limits.h>
+#include <netdb.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <arpa/inet.h>
+#include "xtables.h"
+#include "xshared.h"
+
+#define XTOPT_MKPTR(cb) \
+	((void *)((char *)(cb)->data + (cb)->entry->ptroff))
+
+/**
+ * Creates getopt options from the x6-style option map, and assigns each a
+ * getopt id.
+ */
+struct option *
+xtables_options_xfrm(struct option *orig_opts, struct option *oldopts,
+		     const struct xt_option_entry *entry, unsigned int *offset)
+{
+	unsigned int num_orig, num_old = 0, num_new, i;
+	struct option *merge, *mp;
+
+	if (entry == NULL)
+		return oldopts;
+	for (num_orig = 0; orig_opts[num_orig].name != NULL; ++num_orig)
+		;
+	if (oldopts != NULL)
+		for (num_old = 0; oldopts[num_old].name != NULL; ++num_old)
+			;
+	for (num_new = 0; entry[num_new].name != NULL; ++num_new)
+		;
+
+	/*
+	 * Since @oldopts also has @orig_opts already (and does so at the
+	 * start), skip these entries.
+	 */
+	oldopts += num_orig;
+	num_old -= num_orig;
+
+	merge = malloc(sizeof(*mp) * (num_orig + num_old + num_new + 1));
+	if (merge == NULL)
+		return NULL;
+
+	/* Let the base options -[ADI...] have precedence over everything */
+	memcpy(merge, orig_opts, sizeof(*mp) * num_orig);
+	mp = merge + num_orig;
+
+	/* Second, the new options */
+	xt_params->option_offset += XT_OPTION_OFFSET_SCALE;
+	*offset = xt_params->option_offset;
+
+	for (i = 0; i < num_new; ++i, ++mp, ++entry) {
+		mp->name         = entry->name;
+		mp->has_arg      = entry->type != XTTYPE_NONE;
+		mp->flag         = NULL;
+		mp->val          = entry->id + *offset;
+	}
+
+	/* Third, the old options */
+	memcpy(mp, oldopts, sizeof(*mp) * num_old);
+	mp += num_old;
+	xtables_free_opts(0);
+
+	/* Clear trailing entry */
+	memset(mp, 0, sizeof(*mp));
+	return merge;
+}
+
+static void (*const xtopt_subparse[])(struct xt_option_call *) = {
+	[XTTYPE_NONE]        = NULL,
+};
+
+static const size_t xtopt_psize[] = {
+	[XTTYPE_NONE]        = 0,
+};
+
+/**
+ * The master option parsing routine. May be used for the ".x6_parse"
+ * function pointer in extensions if fully automatic parsing is desired.
+ * It may be also called manually from a custom x6_parse function.
+ */
+void xtables_option_parse(struct xt_option_call *cb)
+{
+	const struct xt_option_entry *entry = cb->entry;
+	unsigned int eflag = 1 << cb->entry->id;
+
+	/*
+	 * With {.id = P_FOO, .excl = P_FOO} we can have simple double-use
+	 * prevention. Though it turned out that this is too much typing (most
+	 * of the options are one-time use only), so now we also have
+	 * %XTOPT_MULTI.
+	 */
+	if ((!(entry->flags & XTOPT_MULTI) || (entry->excl & eflag)) &&
+	    cb->xflags & eflag)
+		xt_params->exit_err(PARAMETER_PROBLEM,
+			"%s: option \"--%s\" can only be used once.\n",
+			cb->ext_name, cb->entry->name);
+	if (cb->invert && !(entry->flags & XTOPT_INVERT))
+		xt_params->exit_err(PARAMETER_PROBLEM,
+			"%s: option \"--%s\" cannot be inverted.\n",
+			cb->ext_name, entry->name);
+	if (entry->type != XTTYPE_NONE && optarg == NULL)
+		xt_params->exit_err(PARAMETER_PROBLEM,
+			"%s: option \"--%s\" requires an argument.\n",
+			cb->ext_name, entry->name);
+	if (entry->type <= ARRAY_SIZE(xtopt_subparse) &&
+	    xtopt_subparse[entry->type] != NULL)
+		xtopt_subparse[entry->type](cb);
+	/* Exclusion with other flags tested later in finalize. */
+	cb->xflags |= 1 << entry->id;
+}
+
+/**
+ * Verifies that an extension's option map descriptor is valid, and ought to
+ * be called right after the extension has been loaded, and before option
+ * merging/xfrm.
+ */
+void xtables_option_metavalidate(const char *name,
+				 const struct xt_option_entry *entry)
+{
+	for (; entry->name != NULL; ++entry) {
+		if (entry->id >= CHAR_BIT * sizeof(unsigned int) ||
+		    entry->id >= XT_OPTION_OFFSET_SCALE)
+			xt_params->exit_err(OTHER_PROBLEM,
+				"Extension %s uses invalid ID %u\n",
+				name, entry->id);
+		if (!(entry->flags & XTOPT_PUT))
+			continue;
+		if (entry->type >= ARRAY_SIZE(xtopt_psize))
+			xt_params->exit_err(OTHER_PROBLEM,
+				"%s: entry type of option \"--%s\" cannot be "
+				"combined with XTOPT_PUT\n",
+				name, entry->name);
+		if (xtopt_psize[entry->type] != entry->size)
+			xt_params->exit_err(OTHER_PROBLEM,
+				"%s: option \"--%s\" points to a memory block "
+				"of wrong size (expected %zu, got %zu)\n",
+				name, entry->name,
+				xtopt_psize[entry->type], entry->size);
+	}
+}
+
+/**
+ * Find an option entry by its id.
+ */
+static const struct xt_option_entry *
+xtables_option_lookup(const struct xt_option_entry *entry, unsigned int id)
+{
+	for (; entry->name != NULL; ++entry)
+		if (entry->id == id)
+			return entry;
+	return NULL;
+}
+
+/**
+ * @c:		getopt id (i.e. with offset)
+ * @fw:		struct ipt_entry or ip6t_entry
+ *
+ * Dispatch arguments to the appropriate parse function, based upon the
+ * extension's choice of API.
+ */
+void xtables_option_tpcall(unsigned int c, char **argv, bool invert,
+			   struct xtables_target *t, void *fw)
+{
+	struct xt_option_call cb;
+
+	if (t->x6_parse == NULL) {
+		if (t->parse != NULL)
+			t->parse(c - t->option_offset, argv, invert,
+				 &t->tflags, fw, &t->t);
+		return;
+	}
+
+	c -= t->option_offset;
+	cb.entry = xtables_option_lookup(t->x6_options, c);
+	if (cb.entry == NULL)
+		xtables_error(OTHER_PROBLEM,
+			"Extension does not know id %u\n", c);
+	cb.arg      = optarg;
+	cb.invert   = invert;
+	cb.ext_name = t->name;
+	cb.data     = t->t->data;
+	cb.xflags   = t->tflags;
+	t->x6_parse(&cb);
+	t->tflags = cb.xflags;
+}
+
+/**
+ * @c:		getopt id (i.e. with offset)
+ * @fw:		struct ipt_entry or ip6t_entry
+ *
+ * Dispatch arguments to the appropriate parse function, based upon the
+ * extension's choice of API.
+ */
+void xtables_option_mpcall(unsigned int c, char **argv, bool invert,
+			   struct xtables_match *m, void *fw)
+{
+	struct xt_option_call cb;
+
+	if (m->x6_parse == NULL) {
+		if (m->parse != NULL)
+			m->parse(c - m->option_offset, argv, invert,
+				 &m->mflags, fw, &m->m);
+		return;
+	}
+
+	c -= m->option_offset;
+	cb.entry = xtables_option_lookup(m->x6_options, c);
+	if (cb.entry == NULL)
+		xtables_error(OTHER_PROBLEM,
+			"Extension does not know id %u\n", c);
+	cb.arg      = optarg;
+	cb.invert   = invert;
+	cb.ext_name = m->name;
+	cb.data     = m->m->data;
+	cb.xflags   = m->mflags;
+	m->x6_parse(&cb);
+	m->mflags = cb.xflags;
+}
+
+/**
+ * @name:	name of extension
+ * @entry:	current option (from all ext's entries) being validated
+ * @xflags:	flags the extension has collected
+ * @i:		conflicting option (id) to test for
+ */
+static void
+xtables_option_fcheck2(const char *name, const struct xt_option_entry *entry,
+		       const struct xt_option_entry *other,
+		       unsigned int xflags)
+{
+	unsigned int ef = 1 << entry->id, of = 1 << other->id;
+
+	if (entry->also & of && !(xflags & of))
+		xt_params->exit_err(PARAMETER_PROBLEM,
+			"%s: option \"--%s\" also requires \"--%s\".\n",
+			name, entry->name, other->name);
+
+	if (!(entry->excl & of))
+		/* Use of entry does not collide with other option, good. */
+		return;
+	if ((xflags & (ef | of)) != (ef | of))
+		/* Conflicting options were not used. */
+		return;
+
+	xt_params->exit_err(PARAMETER_PROBLEM,
+		"%s: option \"--%s\" cannot be used together with \"--%s\".\n",
+		name, entry->name, other->name);
+}
+
+/**
+ * @name:	name of extension
+ * @xflags:	accumulated flags
+ * @entry:	extension's option table
+ *
+ * Check that all option constraints have been met. This effectively replaces
+ * ->final_check of the older API.
+ */
+void xtables_options_fcheck(const char *name, unsigned int xflags,
+			    const struct xt_option_entry *table)
+{
+	const struct xt_option_entry *entry, *other;
+	unsigned int i;
+
+	for (entry = table; entry->name != NULL; ++entry) {
+		if (entry->flags & XTOPT_MAND &&
+		    !(xflags & (1 << entry->id)))
+			xt_params->exit_err(PARAMETER_PROBLEM,
+				"%s: option \"--%s\" must be specified\n",
+				name, entry->name);
+
+		for (i = 0; i < CHAR_BIT * sizeof(entry->id); ++i) {
+			if (entry->id == i)
+				/*
+				 * Avoid conflict with self. Multi-use check
+				 * was done earlier in xtables_option_parse.
+				 */
+				continue;
+			other = xtables_option_lookup(table, i);
+			if (other == NULL)
+				continue;
+			xtables_option_fcheck2(name, entry, other, xflags);
+		}
+	}
+}
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/10] libxt_CHECKSUM: use guided option parser

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_CHECKSUM.c |   46 +++++++++++++-----------------------------
 1 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/extensions/libxt_CHECKSUM.c b/extensions/libxt_CHECKSUM.c
index 83b3d69..df9f9b3 100644
--- a/extensions/libxt_CHECKSUM.c
+++ b/extensions/libxt_CHECKSUM.c
@@ -8,15 +8,14 @@
  *
  * libxt_CHECKSUM.c borrowed some bits from libipt_ECN.c
  */
-#include <stdbool.h>
 #include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <getopt.h>
-
 #include <xtables.h>
 #include <linux/netfilter/xt_CHECKSUM.h>
 
+enum {
+	O_CHECKSUM_FILL = 0,
+};
+
 static void CHECKSUM_help(void)
 {
 	printf(
@@ -24,34 +23,18 @@ static void CHECKSUM_help(void)
 "  --checksum-fill			Fill in packet checksum.\n");
 }
 
-static const struct option CHECKSUM_opts[] = {
-	{.name = "checksum-fill", .has_arg = false, .val = 'F'},
-	XT_GETOPT_TABLEEND,
+static const struct xt_option_entry CHECKSUM_opts[] = {
+	{.name = "checksum-fill", .id = O_CHECKSUM_FILL,
+	 .flags = XTOPT_MAND, .type = XTTYPE_NONE},
+	XTOPT_TABLEEND,
 };
 
-static int CHECKSUM_parse(int c, char **argv, int invert, unsigned int *flags,
-                     const void *entry, struct xt_entry_target **target)
+static void CHECKSUM_parse(struct xt_option_call *cb)
 {
-	struct xt_CHECKSUM_info *einfo
-		= (struct xt_CHECKSUM_info *)(*target)->data;
-
-	switch (c) {
-	case 'F':
-		xtables_param_act(XTF_ONLY_ONCE, "CHECKSUM", "--checksum-fill",
-			*flags & XT_CHECKSUM_OP_FILL);
-		einfo->operation = XT_CHECKSUM_OP_FILL;
-		*flags |= XT_CHECKSUM_OP_FILL;
-		break;
-	}
+	struct xt_CHECKSUM_info *einfo = cb->data;
 
-	return 1;
-}
-
-static void CHECKSUM_check(unsigned int flags)
-{
-	if (!flags)
-		xtables_error(PARAMETER_PROBLEM,
-		           "CHECKSUM target: Parameter --checksum-fill is required");
+	xtables_option_parse(cb);
+	einfo->operation = XT_CHECKSUM_OP_FILL;
 }
 
 static void CHECKSUM_print(const void *ip, const struct xt_entry_target *target,
@@ -82,11 +65,10 @@ static struct xtables_target checksum_tg_reg = {
 	.size		= XT_ALIGN(sizeof(struct xt_CHECKSUM_info)),
 	.userspacesize	= XT_ALIGN(sizeof(struct xt_CHECKSUM_info)),
 	.help		= CHECKSUM_help,
-	.parse		= CHECKSUM_parse,
-	.final_check	= CHECKSUM_check,
 	.print		= CHECKSUM_print,
 	.save		= CHECKSUM_save,
-	.extra_opts	= CHECKSUM_opts,
+	.x6_parse	= CHECKSUM_parse,
+	.x6_options	= CHECKSUM_opts,
 };
 
 void _init(void)
-- 
1.7.1

[PATCH 04/10] libxt_socket: use guided option parser

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_socket.c |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/extensions/libxt_socket.c b/extensions/libxt_socket.c
index e89d1c5..3901649 100644
--- a/extensions/libxt_socket.c
+++ b/extensions/libxt_socket.c
@@ -3,15 +3,17 @@
  *
  * Copyright (C) 2007 BalaBit IT Ltd.
  */
-#include <getopt.h>
-#include <stdbool.h>
 #include <stdio.h>
 #include <xtables.h>
 #include <linux/netfilter/xt_socket.h>
 
-static const struct option socket_mt_opts[] = {
-	{.name = "transparent", .has_arg = false, .val = 't'},
-	XT_GETOPT_TABLEEND,
+enum {
+	O_TRANSPARENT = 0,
+};
+
+static const struct xt_option_entry socket_mt_opts[] = {
+	{.name = "transparent", .id = O_TRANSPARENT, .type = XTTYPE_NONE},
+	XTOPT_TABLEEND,
 };
 
 static void socket_mt_help(void)
@@ -21,17 +23,16 @@ static void socket_mt_help(void)
 		"  --transparent    Ignore non-transparent sockets\n\n");
 }
 
-static int socket_mt_parse(int c, char **argv, int invert, unsigned int *flags,
-			   const void *entry, struct xt_entry_match **match)
+static void socket_mt_parse(struct xt_option_call *cb)
 {
-	struct xt_socket_mtinfo1 *info = (void *)(*match)->data;
+	struct xt_socket_mtinfo1 *info = cb->data;
 
-	switch (c) {
-	case 't':
+	xtables_option_parse(cb);
+	switch (cb->entry->id) {
+	case O_TRANSPARENT:
 		info->flags |= XT_SOCKET_TRANSPARENT;
-		return true;
+		break;
 	}
-	return false;
 }
 
 static void
@@ -68,10 +69,10 @@ static struct xtables_match socket_mt_reg[] = {
 		.size          = XT_ALIGN(sizeof(struct xt_socket_mtinfo1)),
 		.userspacesize = XT_ALIGN(sizeof(struct xt_socket_mtinfo1)),
 		.help          = socket_mt_help,
-		.parse         = socket_mt_parse,
 		.print         = socket_mt_print,
 		.save          = socket_mt_save,
-		.extra_opts    = socket_mt_opts,
+		.x6_parse      = socket_mt_parse,
+		.x6_options    = socket_mt_opts,
 	},
 };
 
-- 
1.7.1

[PATCH 05/10] libxtables: provide better final_check

[Jan Engelhardt]

This passes the per-extension data block to the new x6_fcheck function
pointer, which can then do last alterations without using hacks
like global variables (think libxt_statistic).

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/xtables.h.in |   15 +++++++++++++++
 ip6tables.c          |   18 ++++--------------
 iptables.c           |   18 ++++--------------
 xtoptions.c          |   40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 63 insertions(+), 28 deletions(-)

diff --git a/include/xtables.h.in b/include/xtables.h.in
index 928f465..c281fed 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -103,6 +103,17 @@ struct xt_option_call {
 	} val;
 };
 
+/**
+ * @ext_name:	name of extension currently being processed
+ * @data:	per-extension data block
+ * @xflags:	options of the extension that have been used
+ */
+struct xt_fcheck_call {
+	const char *ext_name;
+	void *data;
+	unsigned int xflags;
+};
+
 /* Include file for additions: new matches and targets. */
 struct xtables_match
 {
@@ -157,6 +168,7 @@ struct xtables_match
 
 	/* New parser */
 	void (*x6_parse)(struct xt_option_call *);
+	void (*x6_fcheck)(struct xt_fcheck_call *);
 	const struct xt_option_entry *x6_options;
 
 	/* Ignore these men behind the curtain: */
@@ -220,6 +232,7 @@ struct xtables_target
 
 	/* New parser */
 	void (*x6_parse)(struct xt_option_call *);
+	void (*x6_fcheck)(struct xt_fcheck_call *);
 	const struct xt_option_entry *x6_options;
 
 	/* Ignore these men behind the curtain: */
@@ -380,6 +393,8 @@ extern void xtables_option_tpcall(unsigned int, char **, bool,
 				  struct xtables_target *, void *);
 extern void xtables_option_mpcall(unsigned int, char **, bool,
 				  struct xtables_match *, void *);
+extern void xtables_option_tfcall(struct xtables_target *);
+extern void xtables_option_mfcall(struct xtables_match *);
 extern void xtables_options_fcheck(const char *, unsigned int,
 				   const struct xt_option_entry *);
 
diff --git a/ip6tables.c b/ip6tables.c
index 83d2fae..3beeddf 100644
--- a/ip6tables.c
+++ b/ip6tables.c
@@ -1782,20 +1782,10 @@ int do_command6(int argc, char *argv[], char **table, struct ip6tc_handle **hand
 		cs.invert = FALSE;
 	}
 
-	for (matchp = cs.matches; matchp; matchp = matchp->next) {
-		if (matchp->match->x6_options != NULL)
-			xtables_options_fcheck(matchp->match->name,
-					       matchp->match->mflags,
-					       matchp->match->x6_options);
-		if (matchp->match->final_check != NULL)
-			matchp->match->final_check(matchp->match->mflags);
-	}
-
-	if (cs.target != NULL && cs.target->x6_options != NULL)
-		xtables_options_fcheck(cs.target->name, cs.target->tflags,
-				       cs.target->x6_options);
-	if (cs.target != NULL && cs.target->final_check != NULL)
-		cs.target->final_check(cs.target->tflags);
+	for (matchp = cs.matches; matchp; matchp = matchp->next)
+		xtables_option_mfcall(matchp->match);
+	if (cs.target != NULL)
+		xtables_option_tfcall(cs.target);
 
 	/* Fix me: must put inverse options checking here --MN */
 
diff --git a/iptables.c b/iptables.c
index 269a66f..2c1528e 100644
--- a/iptables.c
+++ b/iptables.c
@@ -1816,20 +1816,10 @@ int do_command(int argc, char *argv[], char **table, struct iptc_handle **handle
 			"\nThe \"nat\" table is not intended for filtering, "
 		        "the use of DROP is therefore inhibited.\n\n");
 
-	for (matchp = cs.matches; matchp; matchp = matchp->next) {
-		if (matchp->match->x6_options != NULL)
-			xtables_options_fcheck(matchp->match->name,
-					       matchp->match->mflags,
-					       matchp->match->x6_options);
-		if (matchp->match->final_check != NULL)
-			matchp->match->final_check(matchp->match->mflags);
-	}
-
-	if (cs.target != NULL && cs.target->x6_options != NULL)
-		xtables_options_fcheck(cs.target->name, cs.target->tflags,
-				       cs.target->x6_options);
-	if (cs.target != NULL && cs.target->final_check != NULL)
-		cs.target->final_check(cs.target->tflags);
+	for (matchp = cs.matches; matchp; matchp = matchp->next)
+		xtables_option_mfcall(matchp->match);
+	if (cs.target != NULL)
+		xtables_option_tfcall(cs.target);
 
 	/* Fix me: must put inverse options checking here --MN */
 
diff --git a/xtoptions.c b/xtoptions.c
index 3286aa1..df917b6 100644
--- a/xtoptions.c
+++ b/xtoptions.c
@@ -297,3 +297,43 @@ void xtables_options_fcheck(const char *name, unsigned int xflags,
 		}
 	}
 }
+
+/**
+ * Dispatch arguments to the appropriate final_check function, based upon the
+ * extension's choice of API.
+ */
+void xtables_option_tfcall(struct xtables_target *t)
+{
+	if (t->x6_fcheck != NULL) {
+		struct xt_fcheck_call cb;
+
+		cb.ext_name = t->name;
+		cb.data     = t->t->data;
+		cb.xflags   = t->tflags;
+		t->x6_fcheck(&cb);
+	} else if (t->final_check != NULL) {
+		t->final_check(t->tflags);
+	}
+	if (t->x6_options != NULL)
+		xtables_options_fcheck(t->name, t->tflags, t->x6_options);
+}
+
+/**
+ * Dispatch arguments to the appropriate final_check function, based upon the
+ * extension's choice of API.
+ */
+void xtables_option_mfcall(struct xtables_match *m)
+{
+	if (m->x6_fcheck != NULL) {
+		struct xt_fcheck_call cb;
+
+		cb.ext_name = m->name;
+		cb.data     = m->m->data;
+		cb.xflags   = m->mflags;
+		m->x6_fcheck(&cb);
+	} else if (m->final_check != NULL) {
+		m->final_check(m->mflags);
+	}
+	if (m->x6_options != NULL)
+		xtables_options_fcheck(m->name, m->mflags, m->x6_options);
+}
-- 
1.7.1

[PATCH 06/10] libxt_CONNSECMARK: use guided option parser

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_CONNSECMARK.c |   59 +++++++++++++++------------------------
 1 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/extensions/libxt_CONNSECMARK.c b/extensions/libxt_CONNSECMARK.c
index 6b161f3..df2e6b8 100644
--- a/extensions/libxt_CONNSECMARK.c
+++ b/extensions/libxt_CONNSECMARK.c
@@ -5,16 +5,19 @@
  *
  * Copyright (C) 2006 Red Hat, Inc., James Morris <jmorris@redhat.com>
  */
-#include <stdbool.h>
 #include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <getopt.h>
 #include <xtables.h>
 #include <linux/netfilter/xt_CONNSECMARK.h>
 
 #define PFX "CONNSECMARK target: "
 
+enum {
+	O_SAVE = 0,
+	O_RESTORE,
+	F_SAVE    = 1 << O_SAVE,
+	F_RESTORE = 1 << O_RESTORE,
+};
+
 static void CONNSECMARK_help(void)
 {
 	printf(
@@ -23,48 +26,32 @@ static void CONNSECMARK_help(void)
 "  --restore                Copy security mark from connection to packet\n");
 }
 
-static const struct option CONNSECMARK_opts[] = {
-	{.name = "save",    .has_arg = false, .val = '1'},
-	{.name = "restore", .has_arg = false, .val = '2'},
-	XT_GETOPT_TABLEEND,
+static const struct xt_option_entry CONNSECMARK_opts[] = {
+	{.name = "save", .id = O_SAVE, .excl = F_RESTORE, .type = XTTYPE_NONE},
+	{.name = "restore", .id = O_RESTORE, .excl = F_SAVE,
+	 .type = XTTYPE_NONE},
+	XTOPT_TABLEEND,
 };
 
-static int
-CONNSECMARK_parse(int c, char **argv, int invert, unsigned int *flags,
-                  const void *entry, struct xt_entry_target **target)
+static void CONNSECMARK_parse(struct xt_option_call *cb)
 {
-	struct xt_connsecmark_target_info *info =
-		(struct xt_connsecmark_target_info*)(*target)->data;
+	struct xt_connsecmark_target_info *info = cb->data;
 
-	switch (c) {
-	case '1':
-		if (*flags & CONNSECMARK_SAVE)
-			xtables_error(PARAMETER_PROBLEM, PFX
-				   "Can't specify --save twice");
+	xtables_option_parse(cb);
+	switch (cb->entry->id) {
+	case O_SAVE:
 		info->mode = CONNSECMARK_SAVE;
-		*flags |= CONNSECMARK_SAVE;
 		break;
-
-	case '2':
-		if (*flags & CONNSECMARK_RESTORE)
-			xtables_error(PARAMETER_PROBLEM, PFX
-				   "Can't specify --restore twice");
+	case O_RESTORE:
 		info->mode = CONNSECMARK_RESTORE;
-		*flags |= CONNSECMARK_RESTORE;
 		break;
 	}
-
-	return 1;
 }
 
-static void CONNSECMARK_check(unsigned int flags)
+static void CONNSECMARK_check(struct xt_fcheck_call *cb)
 {
-	if (!flags)
+	if (cb->xflags == 0)
 		xtables_error(PARAMETER_PROBLEM, PFX "parameter required");
-
-	if (flags == (CONNSECMARK_SAVE|CONNSECMARK_RESTORE))
-		xtables_error(PARAMETER_PROBLEM, PFX "only one flag of --save "
-		           "or --restore is allowed");
 }
 
 static void print_connsecmark(const struct xt_connsecmark_target_info *info)
@@ -111,12 +98,12 @@ static struct xtables_target connsecmark_target = {
 	.revision	= 0,
 	.size		= XT_ALIGN(sizeof(struct xt_connsecmark_target_info)),
 	.userspacesize	= XT_ALIGN(sizeof(struct xt_connsecmark_target_info)),
-	.parse		= CONNSECMARK_parse,
 	.help		= CONNSECMARK_help,
-	.final_check	= CONNSECMARK_check,
 	.print		= CONNSECMARK_print,
 	.save		= CONNSECMARK_save,
-	.extra_opts	= CONNSECMARK_opts,
+	.x6_parse	= CONNSECMARK_parse,
+	.x6_fcheck	= CONNSECMARK_check,
+	.x6_options	= CONNSECMARK_opts,
 };
 
 void _init(void)
-- 
1.7.1

[PATCH 07/10] libxtables: XTTYPE_UINT32 support

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/xtables.h.in |    4 +++-
 xtoptions.c          |   26 ++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/include/xtables.h.in b/include/xtables.h.in
index c281fed..91a6eaa 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -47,9 +47,11 @@ struct in_addr;
 
 /**
  * %XTTYPE_NONE:	option takes no argument
+ * %XTTYPE_UINT*:	standard integer
  */
 enum xt_option_type {
 	XTTYPE_NONE,
+	XTTYPE_UINT32,
 };
 
 /**
@@ -99,7 +101,7 @@ struct xt_option_call {
 	unsigned int xflags;
 	bool invert;
 	union {
-		/* to be filled */
+		uint32_t u32;
 	} val;
 };
 
diff --git a/xtoptions.c b/xtoptions.c
index df917b6..843395b 100644
--- a/xtoptions.c
+++ b/xtoptions.c
@@ -80,12 +80,34 @@ xtables_options_xfrm(struct option *orig_opts, struct option *oldopts,
 	return merge;
 }
 
+/**
+ * Require a simple integer.
+ */
+static void xtopt_parse_int(struct xt_option_call *cb)
+{
+	const struct xt_option_entry *entry = cb->entry;
+	unsigned int lmin = 0, lmax = UINT32_MAX;
+	unsigned int value;
+
+	if (!xtables_strtoui(cb->arg, NULL, &value, lmin, lmax))
+		xt_params->exit_err(PARAMETER_PROBLEM,
+			"%s: bad value for option \"--%s\", "
+			"or out of range (%u-%u).\n",
+			cb->ext_name, entry->name, lmin, lmax);
+
+	if (entry->type == XTTYPE_UINT32) {
+		cb->val.u32 = value;
+		if (entry->flags & XTOPT_PUT)
+			*(uint32_t *)XTOPT_MKPTR(cb) = cb->val.u32;
+	}
+}
+
 static void (*const xtopt_subparse[])(struct xt_option_call *) = {
-	[XTTYPE_NONE]        = NULL,
+	[XTTYPE_UINT32]      = xtopt_parse_int,
 };
 
 static const size_t xtopt_psize[] = {
-	[XTTYPE_NONE]        = 0,
+	[XTTYPE_UINT32]      = sizeof(uint32_t),
 };
 
 /**
-- 
1.7.1

[PATCH 08/10] libxt_cpu: use guided option parser

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_cpu.c |   65 ++++++++++++------------------------------------
 1 files changed, 16 insertions(+), 49 deletions(-)

diff --git a/extensions/libxt_cpu.c b/extensions/libxt_cpu.c
index 77efec7..404a6a6 100644
--- a/extensions/libxt_cpu.c
+++ b/extensions/libxt_cpu.c
@@ -1,13 +1,11 @@
-/* Shared library add-on to iptables to add CPU match support. */
-#include <stdbool.h>
 #include <stdio.h>
-#include <netdb.h>
-#include <string.h>
-#include <stdlib.h>
-#include <getopt.h>
 #include <xtables.h>
 #include <linux/netfilter/xt_cpu.h>
 
+enum {
+	O_CPU = 0,
+};
+
 static void cpu_help(void)
 {
 	printf(
@@ -15,50 +13,20 @@ static void cpu_help(void)
 "[!] --cpu number   Match CPU number\n");
 }
 
-static const struct option cpu_opts[] = {
-	{.name = "cpu", .has_arg = true, .val = '1'},
-	XT_GETOPT_TABLEEND,
+static const struct xt_option_entry cpu_opts[] = {
+	{.name = "cpu", .id = O_CPU, .type = XTTYPE_UINT32,
+	 .flags = XTOPT_INVERT | XTOPT_MAND | XTOPT_PUT,
+	 XTOPT_POINTER(struct xt_cpu_info, cpu)},
+	XTOPT_TABLEEND,
 };
 
-static void
-parse_cpu(const char *s, struct xt_cpu_info *info)
+static void cpu_parse(struct xt_option_call *cb)
 {
-	unsigned int cpu;
-	char *end;
-
-	if (!xtables_strtoui(s, &end, &cpu, 0, UINT32_MAX))
-		xtables_param_act(XTF_BAD_VALUE, "cpu", "--cpu", s);
-
-	if (*end != '\0')
-		xtables_param_act(XTF_BAD_VALUE, "cpu", "--cpu", s);
-
-	info->cpu = cpu;
-}
+	struct xt_cpu_info *cpuinfo = cb->data;
 
-static int
-cpu_parse(int c, char **argv, int invert, unsigned int *flags,
-          const void *entry, struct xt_entry_match **match)
-{
-	struct xt_cpu_info *cpuinfo = (struct xt_cpu_info *)(*match)->data;
-
-	switch (c) {
-	case '1':
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		parse_cpu(optarg, cpuinfo);
-		if (invert)
-			cpuinfo->invert = 1;
-		*flags = 1;
-		break;
-	}
-
-	return 1;
-}
-
-static void cpu_check(unsigned int flags)
-{
-	if (!flags)
-		xtables_error(PARAMETER_PROBLEM,
-			      "You must specify `--cpu'");
+	xtables_option_parse(cb);
+	if (cb->invert)
+		cpuinfo->invert = true;
 }
 
 static void
@@ -83,11 +51,10 @@ static struct xtables_match cpu_match = {
 	.size		= XT_ALIGN(sizeof(struct xt_cpu_info)),
 	.userspacesize	= XT_ALIGN(sizeof(struct xt_cpu_info)),
 	.help		= cpu_help,
-	.parse		= cpu_parse,
-	.final_check	= cpu_check,
 	.print		= cpu_print,
 	.save		= cpu_save,
-	.extra_opts	= cpu_opts,
+	.x6_parse	= cpu_parse,
+	.x6_options	= cpu_opts,
 };
 
 void _init(void)
-- 
1.7.1

[PATCH 09/10] libxtables: min-max option support

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 include/xtables.h.in |    3 +++
 xtoptions.c          |    5 +++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/xtables.h.in b/include/xtables.h.in
index 91a6eaa..14d7b04 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -76,6 +76,8 @@ enum xt_option_flags {
  * @flags:	bitmask of option flags, see %XTOPT_*
  * @ptroff:	offset into private structure for member
  * @size:	size of the item pointed to by @ptroff; this is a safeguard
+ * @min:	lowest allowed value (for singular integral types)
+ * @max:	highest allowed value (for singular integral types)
  */
 struct xt_option_entry {
 	const char *name;
@@ -83,6 +85,7 @@ struct xt_option_entry {
 	unsigned int id, excl, also, flags;
 	unsigned int ptroff;
 	size_t size;
+	unsigned int min, max;
 };
 
 /**
diff --git a/xtoptions.c b/xtoptions.c
index 843395b..6a119ec 100644
--- a/xtoptions.c
+++ b/xtoptions.c
@@ -89,6 +89,11 @@ static void xtopt_parse_int(struct xt_option_call *cb)
 	unsigned int lmin = 0, lmax = UINT32_MAX;
 	unsigned int value;
 
+	if (cb->entry->min != 0)
+		lmin = cb->entry->min;
+	if (cb->entry->max != 0)
+		lmax = cb->entry->max;
+
 	if (!xtables_strtoui(cb->arg, NULL, &value, lmin, lmax))
 		xt_params->exit_err(PARAMETER_PROBLEM,
 			"%s: bad value for option \"--%s\", "
-- 
1.7.1

[PATCH 10/10] libxt_cluster: use guided option parser

[Jan Engelhardt]

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_cluster.c |  187 +++++++++++--------------------------------
 1 files changed, 48 insertions(+), 139 deletions(-)

diff --git a/extensions/libxt_cluster.c b/extensions/libxt_cluster.c
index e1607d0..3adff12 100644
--- a/extensions/libxt_cluster.c
+++ b/extensions/libxt_cluster.c
@@ -5,21 +5,10 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-#include <stdbool.h>
 #include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <getopt.h>
-#include <stddef.h>
-
 #include <xtables.h>
-#include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_cluster.h>
 
-/* hack to keep for check */
-static unsigned int total_nodes;
-static unsigned int node_mask;
-
 static void
 cluster_help(void)
 {
@@ -32,160 +21,80 @@ cluster_help(void)
 }
 
 enum {
-	CLUSTER_OPT_TOTAL_NODES,
-	CLUSTER_OPT_LOCAL_NODE,
-	CLUSTER_OPT_NODE_MASK,
-	CLUSTER_OPT_HASH_SEED,
+	O_CL_TOTAL_NODES = 0,
+	O_CL_LOCAL_NODE,
+	O_CL_LOCAL_NODEMASK,
+	O_CL_HASH_SEED,
+	F_CL_TOTAL_NODES    = 1 << O_CL_TOTAL_NODES,
+	F_CL_LOCAL_NODE     = 1 << O_CL_LOCAL_NODE,
+	F_CL_LOCAL_NODEMASK = 1 << O_CL_LOCAL_NODEMASK,
+	F_CL_HASH_SEED      = 1 << O_CL_HASH_SEED,
 };
 
-static const struct option cluster_opts[] = {
-	{.name = "cluster-total-nodes",    .has_arg = true, .val = CLUSTER_OPT_TOTAL_NODES},
-	{.name = "cluster-local-node",     .has_arg = true, .val = CLUSTER_OPT_LOCAL_NODE},
-	{.name = "cluster-local-nodemask", .has_arg = true, .val = CLUSTER_OPT_NODE_MASK},
-	{.name = "cluster-hash-seed",      .has_arg = true, .val = CLUSTER_OPT_HASH_SEED},
-	XT_GETOPT_TABLEEND,
+#define s struct xt_cluster_match_info
+static const struct xt_option_entry cluster_opts[] = {
+	{.name = "cluster-total-nodes", .id = O_CL_TOTAL_NODES,
+	 .type = XTTYPE_UINT32, .min = 1, .max = XT_CLUSTER_NODES_MAX,
+	 .flags = XTOPT_MAND | XTOPT_PUT, XTOPT_POINTER(s, total_nodes)},
+	{.name = "cluster-local-node", .id = O_CL_LOCAL_NODE,
+	 .excl = F_CL_LOCAL_NODEMASK, .flags = XTOPT_INVERT,
+	 .type = XTTYPE_UINT32, .min = 1, .max = XT_CLUSTER_NODES_MAX},
+	{.name = "cluster-local-nodemask", .id = O_CL_LOCAL_NODEMASK,
+	 .excl = F_CL_LOCAL_NODE, .type = XTTYPE_UINT32,
+	 .min = 1, .max = XT_CLUSTER_NODES_MAX,
+	 .flags = XTOPT_INVERT | XTOPT_PUT, XTOPT_POINTER(s, node_mask)},
+	{.name = "cluster-hash-seed", .id = O_CL_HASH_SEED,
+	 .type = XTTYPE_UINT32, .flags = XTOPT_MAND | XTOPT_PUT,
+	 XTOPT_POINTER(s, hash_seed)},
+	XTOPT_TABLEEND,
 };
 
-static int 
-cluster_parse(int c, char **argv, int invert, unsigned int *flags,
-	      const void *entry, struct xt_entry_match **match)
+static void cluster_parse(struct xt_option_call *cb)
 {
-	struct xt_cluster_match_info *info = (void *)(*match)->data;
-	unsigned int num;
+	struct xt_cluster_match_info *info = cb->data;
 
-	switch (c) {
-	case CLUSTER_OPT_TOTAL_NODES:
-		if (*flags & (1 << c)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Can only specify "
-				      "`--cluster-total-nodes' once");
-		}
-		if (!xtables_strtoui(optarg, NULL, &num, 1,
-				     XT_CLUSTER_NODES_MAX)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Unable to parse `%s' in "
-				      "`--cluster-total-nodes'", optarg);
-		}
-		total_nodes = num;
-		info->total_nodes = total_nodes = num;
-		*flags |= 1 << c;
-		break;
-	case CLUSTER_OPT_LOCAL_NODE:
-		if (*flags & (1 << c)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Can only specify "
-				      "`--cluster-local-node' once");
-		}
-		if (*flags & (1 << CLUSTER_OPT_NODE_MASK)) {
-			xtables_error(PARAMETER_PROBLEM, "You cannot use "
-				      "`--cluster-local-nodemask' and "
-				      "`--cluster-local-node'");
-		}
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-
-		if (!xtables_strtoui(optarg, NULL, &num, 1,
-				     XT_CLUSTER_NODES_MAX)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Unable to parse `%s' in "
-				      "`--cluster-local-node'", optarg);
-		}
-		if (invert)
+	xtables_option_parse(cb);
+	switch (cb->entry->id) {
+	case O_CL_LOCAL_NODE:
+		if (cb->invert)
 			info->flags |= XT_CLUSTER_F_INV;
-
-		info->node_mask = node_mask = (1 << (num - 1));
-		*flags |= 1 << c;
+		info->node_mask = 1 << (cb->val.u32 - 1);
 		break;
-	case CLUSTER_OPT_NODE_MASK:
-		if (*flags & (1 << c)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Can only specify "
-				      "`--cluster-local-node' once");
-		}
-		if (*flags & (1 << CLUSTER_OPT_LOCAL_NODE)) {
-			xtables_error(PARAMETER_PROBLEM, "You cannot use "
-				      "`--cluster-local-nodemask' and "
-				      "`--cluster-local-node'");
-		}
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-
-		if (!xtables_strtoui(optarg, NULL, &num, 1,
-				     XT_CLUSTER_NODES_MAX)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Unable to parse `%s' in "
-				      "`--cluster-local-node'", optarg);
-		}
-		if (invert)
+	case O_CL_LOCAL_NODEMASK:
+		if (cb->invert)
 			info->flags |= XT_CLUSTER_F_INV;
-
-		info->node_mask = node_mask = num;
-		*flags |= 1 << c;
-		break;
-
-	case CLUSTER_OPT_HASH_SEED:
-		if (*flags & (1 << c)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Can only specify "
-				      "`--cluster-hash-seed' once");
-		}
-		if (!xtables_strtoui(optarg, NULL, &num, 0, UINT32_MAX)) {
-			xtables_error(PARAMETER_PROBLEM,
-				      "Unable to parse `%s'", optarg);
-		}
-		info->hash_seed = num;
-		*flags |= 1 << c;
 		break;
 	}
-
-	return 1;
 }
 
-static void
-cluster_check(unsigned int flags)
+static void cluster_check(struct xt_fcheck_call *cb)
 {
-	if ((flags & ((1 << CLUSTER_OPT_TOTAL_NODES) |
-		      (1 << CLUSTER_OPT_LOCAL_NODE) |
-		      (1 << CLUSTER_OPT_HASH_SEED)))
-		== ((1 << CLUSTER_OPT_TOTAL_NODES) |
-		    (1 << CLUSTER_OPT_LOCAL_NODE) |
-		    (1 << CLUSTER_OPT_HASH_SEED))) {
-		if (node_mask >= (1ULL << total_nodes)) {
+	const struct xt_cluster_match_info *info = cb->data;
+	unsigned int test;
+
+	test = F_CL_TOTAL_NODES | F_CL_LOCAL_NODE | F_CL_HASH_SEED;
+	if ((cb->xflags & test) == test) {
+		if (info->node_mask >= (1ULL << info->total_nodes))
 			xtables_error(PARAMETER_PROBLEM,
 				      "cluster match: "
 				      "`--cluster-local-node' "
 				      "must be <= `--cluster-total-nodes'");
-		}
 		return;
 	}
-	if ((flags & ((1 << CLUSTER_OPT_TOTAL_NODES) |
-		      (1 << CLUSTER_OPT_NODE_MASK) |
-		      (1 << CLUSTER_OPT_HASH_SEED)))
-		== ((1 << CLUSTER_OPT_TOTAL_NODES) |
-		    (1 << CLUSTER_OPT_NODE_MASK) |
-		    (1 << CLUSTER_OPT_HASH_SEED))) {
-		if (node_mask >= (1ULL << total_nodes)) {
+
+	test = F_CL_TOTAL_NODES | F_CL_LOCAL_NODEMASK | F_CL_HASH_SEED;
+	if ((cb->xflags & test) == test) {
+		if (info->node_mask >= (1ULL << info->total_nodes))
 			xtables_error(PARAMETER_PROBLEM,
 				      "cluster match: "
 				      "`--cluster-local-nodemask' too big "
 				      "for `--cluster-total-nodes'");
-		}
 		return;
 	}
-	if (!(flags & (1 << CLUSTER_OPT_TOTAL_NODES))) {
-		xtables_error(PARAMETER_PROBLEM,
-			      "cluster match: `--cluster-total-nodes' "
-			      "is missing");
-	}
-	if (!(flags & (1 << CLUSTER_OPT_HASH_SEED))) {
-		xtables_error(PARAMETER_PROBLEM,
-			      "cluster match: `--cluster-hash-seed' "
-			      "is missing");
-	}
-	if (!(flags & ((1 << (CLUSTER_OPT_LOCAL_NODE) |
-		       (1 << (CLUSTER_OPT_NODE_MASK)))))) {
+	if (!(cb->xflags & (F_CL_LOCAL_NODE | F_CL_LOCAL_NODEMASK)))
 		xtables_error(PARAMETER_PROBLEM,
 			      "cluster match: `--cluster-local-node' or"
 			      "`--cluster-local-nodemask' is missing");
-	}
 }
 
 static void
@@ -224,11 +133,11 @@ static struct xtables_match cluster_mt_reg = {
 	.size		= XT_ALIGN(sizeof(struct xt_cluster_match_info)),
 	.userspacesize  = XT_ALIGN(sizeof(struct xt_cluster_match_info)),
  	.help		= cluster_help,
-	.parse		= cluster_parse,
-	.final_check	= cluster_check,
 	.print		= cluster_print,
 	.save		= cluster_save,
-	.extra_opts	= cluster_opts,
+	.x6_parse	= cluster_parse,
+	.x6_fcheck	= cluster_check,
+	.x6_options	= cluster_opts,
 };
 
 void _init(void)
-- 
1.7.1

Re: Guided option parser

[Patrick McHardy]

On 06.04.2011 13:31, Jan Engelhardt wrote:
> This is a staggered submit, with 10 patches in the first submission.
> See 01/10 for a description.
> 
> Currently, the entire set of 64 has a diffstat of
> 
>  74 files changed, 2845 insertions(+), 4062 deletions(-)
> 
> this is roughly a third of the entire work, and it is expected this
> to linearly continue until ~200 patches. But I felt I needed to flush
> the queue now.
> 
> ===
> The following changes since commit a239728ec064666025de2723997d87b176d57fd6:
> 
>   mark newly opened fds as FD_CLOEXEC (close on exec) (2011-04-04 15:30:32 +0200)
> 
> are available in the git repository at:
>   git://dev.medozas.de/iptables opts
> 
> Jan Engelhardt (10):
>       extensions: add missing checks for specific flags (2)
>       libxtables: guided option parser
>       libxt_CHECKSUM: use guided option parser
>       libxt_socket: use guided option parser
>       libxtables: provide better final_check
>       libxt_CONNSECMARK: use guided option parser
>       libxtables: XTTYPE_UINT32 support
>       libxt_cpu: use guided option parser
>       libxtables: min-max option support
>       libxt_cluster: use guided option parser

Looks good to me. Pulled, thanks Jan.