Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 1 Aug 2016 15:03:52 +0200
From:      Christian Mauderer <christian.mauderer@embedded-brains.de>
To:        "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>
Subject:   Re: Changes to pfctl to allow easier integration into a library
Message-ID:  <cb7c6bb6-8160-3524-5432-d4ff46e16af5@embedded-brains.de>
In-Reply-To: <25df9fd5-be75-b9ae-aa3a-22abef3bddf0@embedded-brains.de>
References:  <25df9fd5-be75-b9ae-aa3a-22abef3bddf0@embedded-brains.de>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------D946645D9839002572096E98
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Am 28.07.2016 um 16:03 schrieb Christian Mauderer:
> Hello,
>=20
[...]
>=20
> Would the attached patches be acceptable for integration into the
> FreeBSD sources?
>=20
> I've generated the patches against the git commit b6ff7c02cf9 on
> https://github.com/freebsd/freebsd.git. Please tell me if another form
> would be better.
>=20
> Kind regards,
>=20
> Christian Mauderer
>=20

Hello,

I've got one additional patch: I made most of the global variables
static. They are used only inside the scope of one single c file.
Despite that, they have not been static. If I try to link the source
file into a library every non-static variables pollutes my name space.

Can I improve anything to make the patches more acceptable?

Is the virtualisation that Bjoern mentioned necessary or was my
interpretation correct that this is only meant for kernel space code?

Kind Regards

Christian Mauderer
--=20
--------------------------------------------
embedded brains GmbH
Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer@embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine gesch=C3=A4ftliche Mitteilung im Sinne des EHUG=
.

--------------D946645D9839002572096E98
Content-Type: text/x-patch;
 name="0004-pfctl-Make-most-global-variables-static.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
 filename="0004-pfctl-Make-most-global-variables-static.patch"

=46rom 6bf9828e65e73bd203a3e6d5081e27160e5e5ea0 Mon Sep 17 00:00:00 2001
From: Christian Mauderer <Christian.Mauderer@embedded-brains.de>
Date: Fri, 29 Jul 2016 17:03:12 +0200
Subject: [PATCH 4/4] pfctl: Make most global variables static.

This will make it easier to link as a library.
---
 sbin/pfctl/parse.y          | 36 +++++++++++++++++------------------
 sbin/pfctl/pfctl.c          | 46 ++++++++++++++++++++++-----------------=
------
 sbin/pfctl/pfctl_altq.c     |  4 ++--
 sbin/pfctl/pfctl_optimize.c | 11 ++++++-----
 sbin/pfctl/pfctl_osfp.c     |  6 +++---
 sbin/pfctl/pfctl_parser.c   |  2 +-
 6 files changed, 53 insertions(+), 52 deletions(-)

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index e0cfa3d..676578f 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -80,7 +80,7 @@ static int		 blockpolicy =3D PFRULE_DROP;
 static int		 require_order =3D 1;
 static int		 default_statelock;
=20
-TAILQ_HEAD(files, file)		 files =3D TAILQ_HEAD_INITIALIZER(files);
+static TAILQ_HEAD(files, file)	 files =3D TAILQ_HEAD_INITIALIZER(files);=

 static struct file {
 	TAILQ_ENTRY(file)	 entry;
 	FILE			*stream;
@@ -100,7 +100,7 @@ int		 lgetc(int);
 int		 lungetc(int);
 int		 findeol(void);
=20
-TAILQ_HEAD(symhead, sym)	 symhead =3D TAILQ_HEAD_INITIALIZER(symhead);
+static TAILQ_HEAD(symhead, sym)	 symhead =3D TAILQ_HEAD_INITIALIZER(symh=
ead);
 struct sym {
 	TAILQ_ENTRY(sym)	 entry;
 	int			 used;
@@ -196,7 +196,7 @@ struct peer {
 	struct node_port	*port;
 };
=20
-struct node_queue {
+static struct node_queue {
 	char			 queue[PF_QNAME_SIZE];
 	char			 parent[PF_QNAME_SIZE];
 	char			 ifname[IFNAMSIZ];
@@ -210,7 +210,7 @@ struct node_qassign {
 	char		*pqname;
 };
=20
-struct filter_opts {
+static struct filter_opts {
 	int			 marker;
 #define FOM_FLAGS	0x01
 #define FOM_ICMP	0x02
@@ -250,12 +250,12 @@ struct filter_opts {
 	}			 divert;
 } filter_opts;
=20
-struct antispoof_opts {
+static struct antispoof_opts {
 	char			*label;
 	u_int			 rtableid;
 } antispoof_opts;
=20
-struct scrub_opts {
+static struct scrub_opts {
 	int			 marker;
 #define SOM_MINTTL	0x01
 #define SOM_MAXMSS	0x02
@@ -273,7 +273,7 @@ struct scrub_opts {
 	u_int			 rtableid;
 } scrub_opts;
=20
-struct queue_opts {
+static struct queue_opts {
 	int			marker;
 #define QOM_BWSPEC	0x01
 #define QOM_SCHEDULER	0x02
@@ -287,13 +287,13 @@ struct queue_opts {
 	int			qlimit;
 } queue_opts;
=20
-struct table_opts {
+static struct table_opts {
 	int			flags;
 	int			init_addr;
 	struct node_tinithead	init_nodes;
 } table_opts;
=20
-struct pool_opts {
+static struct pool_opts {
 	int			 marker;
 #define POM_TYPE		0x01
 #define POM_STICKYADDRESS	0x02
@@ -304,10 +304,10 @@ struct pool_opts {
=20
 } pool_opts;
=20
-struct codel_opts	 codel_opts;
-struct node_hfsc_opts	 hfsc_opts;
-struct node_fairq_opts	 fairq_opts;
-struct node_state_opt	*keep_state_defaults =3D NULL;
+static struct codel_opts	 codel_opts;
+static struct node_hfsc_opts	 hfsc_opts;
+static struct node_fairq_opts	 fairq_opts;
+static struct node_state_opt	*keep_state_defaults =3D NULL;
=20
 int		 disallow_table(struct node_host *, const char *);
 int		 disallow_urpf_failed(struct node_host *, const char *);
@@ -352,7 +352,7 @@ void	 remove_invalid_hosts(struct node_host **, sa_fa=
mily_t *);
 int	 invalid_redirect(struct node_host *, sa_family_t);
 u_int16_t parseicmpspec(char *, sa_family_t);
=20
-TAILQ_HEAD(loadanchorshead, loadanchors)
+static TAILQ_HEAD(loadanchorshead, loadanchors)
     loadanchorshead =3D TAILQ_HEAD_INITIALIZER(loadanchorshead);
=20
 struct loadanchors {
@@ -5572,10 +5572,10 @@ lookup(char *s)
=20
 #define MAXPUSHBACK	128
=20
-char	*parsebuf;
-int	 parseindex;
-char	 pushback_buffer[MAXPUSHBACK];
-int	 pushback_index =3D 0;
+static char	*parsebuf;
+static int	 parseindex;
+static char	 pushback_buffer[MAXPUSHBACK];
+static int	 pushback_index =3D 0;
=20
 int
 lgetc(int quotec)
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index a95ac23..9cad2b1 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -102,29 +102,29 @@ int	 pfctl_load_ruleset(struct pfctl *, char *,
 int	 pfctl_load_rule(struct pfctl *, char *, struct pf_rule *, int);
 const char	*pfctl_lookup_option(char *, const char * const *);
=20
-struct pf_anchor_global	 pf_anchors;
-struct pf_anchor	 pf_main_anchor;
-
-const char	*clearopt;
-char		*rulesopt;
-const char	*showopt;
-const char	*debugopt;
-char		*anchoropt;
-const char	*optiopt =3D NULL;
-const char	*pf_device =3D "/dev/pf";
-char		*ifaceopt;
-char		*tableopt;
-const char	*tblcmdopt;
-int		 src_node_killers;
-char		*src_node_kill[2];
-int		 state_killers;
-char		*state_kill[2];
-int		 loadopt;
-int		 altqsupport;
-
-int		 dev =3D -1;
-int		 first_title =3D 1;
-int		 labels =3D 0;
+static struct pf_anchor_global	 pf_anchors;
+static struct pf_anchor	 pf_main_anchor;
+
+static const char	*clearopt;
+static char		*rulesopt;
+static const char	*showopt;
+static const char	*debugopt;
+static char		*anchoropt;
+static const char	*optiopt =3D NULL;
+static const char	*pf_device =3D "/dev/pf";
+static char		*ifaceopt;
+static char		*tableopt;
+static const char	*tblcmdopt;
+static int		 src_node_killers;
+static char		*src_node_kill[2];
+static int		 state_killers;
+static char		*state_kill[2];
+int			 loadopt;
+int			 altqsupport;
+
+int			 dev =3D -1;
+static int		 first_title =3D 1;
+static int		 labels =3D 0;
=20
 #define INDENT(d, o)	do {						\
 				if (o) {				\
diff --git a/sbin/pfctl/pfctl_altq.c b/sbin/pfctl/pfctl_altq.c
index eed3173..1e3569d 100644
--- a/sbin/pfctl/pfctl_altq.c
+++ b/sbin/pfctl/pfctl_altq.c
@@ -50,8 +50,8 @@ __FBSDID("$FreeBSD$");
=20
 #define is_sc_null(sc)	(((sc) =3D=3D NULL) || ((sc)->m1 =3D=3D 0 && (sc)=
->m2 =3D=3D 0))
=20
-TAILQ_HEAD(altqs, pf_altq) altqs =3D TAILQ_HEAD_INITIALIZER(altqs);
-LIST_HEAD(gen_sc, segment) rtsc, lssc;
+static TAILQ_HEAD(altqs, pf_altq) altqs =3D TAILQ_HEAD_INITIALIZER(altqs=
);
+static LIST_HEAD(gen_sc, segment) rtsc, lssc;
=20
 struct pf_altq	*qname_to_pfaltq(const char *, const char *);
 u_int32_t	 qname_to_qid(const char *);
diff --git a/sbin/pfctl/pfctl_optimize.c b/sbin/pfctl/pfctl_optimize.c
index 0f89b22..1552185 100644
--- a/sbin/pfctl/pfctl_optimize.c
+++ b/sbin/pfctl/pfctl_optimize.c
@@ -90,7 +90,7 @@ enum {
     COMBINED,	/* the field may itself be combined with other rules */
     DC,		/* we just don't care about the field */
     NEVER};	/* we should never see this field set?!? */
-struct pf_rule_field {
+static struct pf_rule_field {
 	const char	*prf_name;
 	int		 prf_type;
 	size_t		 prf_offset;
@@ -244,8 +244,9 @@ int	superblock_inclusive(struct superblock *, struct =
pf_opt_rule *);
 void	superblock_free(struct pfctl *, struct superblock *);
=20
=20
-int (*skip_comparitors[PF_SKIP_COUNT])(struct pf_rule *, struct pf_rule =
*);
-const char *skip_comparitors_names[PF_SKIP_COUNT];
+static int (*skip_comparitors[PF_SKIP_COUNT])(struct pf_rule *,
+    struct pf_rule *);
+static const char *skip_comparitors_names[PF_SKIP_COUNT];
 #define PF_SKIP_COMPARITORS {				\
     { "ifp", PF_SKIP_IFP, skip_cmp_ifp },		\
     { "dir", PF_SKIP_DIR, skip_cmp_dir },		\
@@ -257,8 +258,8 @@ const char *skip_comparitors_names[PF_SKIP_COUNT];
     { "dport", PF_SKIP_DST_PORT, skip_cmp_dst_port }	\
 }
=20
-struct pfr_buffer table_buffer;
-int table_identifier;
+static struct pfr_buffer table_buffer;
+static int table_identifier;
=20
=20
 int
diff --git a/sbin/pfctl/pfctl_osfp.c b/sbin/pfctl/pfctl_osfp.c
index df78981..649c1e8 100644
--- a/sbin/pfctl/pfctl_osfp.c
+++ b/sbin/pfctl/pfctl_osfp.c
@@ -64,9 +64,9 @@ struct name_entry {
 	struct name_list	nm_sublist;
 	int			nm_sublist_num;
 };
-struct name_list classes =3D LIST_HEAD_INITIALIZER(&classes);
-int class_count;
-int fingerprint_count;
+static struct name_list classes =3D LIST_HEAD_INITIALIZER(&classes);
+static int class_count;
+static int fingerprint_count;
=20
 void			 add_fingerprint(int, int, struct pf_osfp_ioctl *);
 struct name_entry	*fingerprint_name_entry(struct name_list *, char *);
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index d9aa49a..f7a7ad9 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1147,7 +1147,7 @@ check_netmask(struct node_host *h, sa_family_t af)
=20
 /* interface lookup routines */
=20
-struct node_host	*iftab;
+static struct node_host	*iftab;
=20
 void
 ifa_load(void)
--=20
2.9.2


--------------D946645D9839002572096E98--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?cb7c6bb6-8160-3524-5432-d4ff46e16af5>