Bye bye atexit(), bye bye globals...

The pfctl.c part will probably need some further improvements.
ok henning@

4 files changed, 47 insertions, 65 deletions

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 224a0e3339e..4a3640737ac 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.394 2003/07/03 09:13:05 cedric Exp $	*/
+/*	$OpenBSD: parse.y,v 1.395 2003/07/03 21:09:13 cedric Exp $	*/
 
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -858,11 +858,12 @@ tabledef	: TABLE '<' STRING '>' table_opts {
 			if (pfctl_define_table($3, $5.flags, $5.init_addr,
 			    (pf->opts & PF_OPT_NOACTION) || !(pf->loadopt &
 				(PFCTL_FLAG_TABLE | PFCTL_FLAG_ALL)),
-				pf->anchor, pf->ruleset, pf->ab)) {
+			    pf->anchor, pf->ruleset, pf->ab, pf->tticket)) {
 				yyerror("cannot define table %s: %s", $3,
 				    pfr_strerror(errno));
 				YYERROR;				
 			}
+			pf->tdirty = 1;
 		}
 		;
 
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 57e313bc4fd..6769b1a2be0 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl.c,v 1.179 2003/07/03 09:13:06 cedric Exp $ */
+/*	$OpenBSD: pfctl.c,v 1.180 2003/07/03 21:09:13 cedric Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -914,6 +914,9 @@ int
 pfctl_rules(int dev, char *filename, int opts, char *anchorname,
     char *rulesetname)
 {
+#define ERR(x) do { warn(x); goto _error; } while(0)
+#define ERRX(x) do { warnx(x); goto _error; } while(0)
+
 	FILE *fin;
 	struct pfioc_rule	pr[PF_RULESET_MAX];
 	struct pfioc_altq	pa;
@@ -944,29 +947,31 @@ pfctl_rules(int dev, char *filename, int opts, char *anchorname,
 		if ((loadopt & (PFCTL_FLAG_NAT | PFCTL_FLAG_ALL)) != 0) {
 			pr[PF_RULESET_NAT].rule.action = PF_NAT;
 			if (ioctl(dev, DIOCBEGINRULES, &pr[PF_RULESET_NAT]))
-				err(1, "DIOCBEGINRULES");
+				ERR("DIOCBEGINRULES");
 			pr[PF_RULESET_RDR].rule.action = PF_RDR;
 			if (ioctl(dev, DIOCBEGINRULES, &pr[PF_RULESET_RDR]))
-				err(1, "DIOCBEGINRULES");
+				ERR("DIOCBEGINRULES");
 			pr[PF_RULESET_BINAT].rule.action = PF_BINAT;
 			if (ioctl(dev, DIOCBEGINRULES, &pr[PF_RULESET_BINAT]))
-				err(1, "DIOCBEGINRULES");
+				ERR("DIOCBEGINRULES");
 		}
 		if (((altqsupport && (loadopt &
 		    (PFCTL_FLAG_ALTQ | PFCTL_FLAG_ALL)) != 0)) &&
 		    ioctl(dev, DIOCBEGINALTQS, &pa.ticket)) {
-			err(1, "DIOCBEGINALTQS");
+			ERR("DIOCBEGINALTQS");
 		}
 		if ((loadopt & (PFCTL_FLAG_FILTER | PFCTL_FLAG_ALL)) != 0) {
 			pr[PF_RULESET_SCRUB].rule.action = PF_SCRUB;
 			if (ioctl(dev, DIOCBEGINRULES, &pr[PF_RULESET_SCRUB]))
-				err(1, "DIOCBEGINRULES");
+				ERR("DIOCBEGINRULES");
 			pr[PF_RULESET_FILTER].rule.action = PF_PASS;
 			if (ioctl(dev, DIOCBEGINRULES, &pr[PF_RULESET_FILTER]))
-				err(1, "DIOCBEGINRULES");
+				ERR("DIOCBEGINRULES");
+		}
+		if (loadopt & (PFCTL_FLAG_TABLE | PFCTL_FLAG_ALL)) {
+		        if (pfr_ina_begin(&pf.tticket, NULL, 0) != 0)
+				ERR("begin table");
 		}
-		if (loadopt & (PFCTL_FLAG_TABLE | PFCTL_FLAG_ALL))
-			pfctl_begin_table();
 	}
 	/* fill in callback data */
 	pf.dev = dev;
@@ -981,41 +986,44 @@ pfctl_rules(int dev, char *filename, int opts, char *anchorname,
 	pf.anchor = anchorname;
 	pf.ruleset = rulesetname;
 	if (parse_rules(fin, &pf) < 0)
-		errx(1, "Syntax error in config file: pf rules not loaded");
+		ERRX("Syntax error in config file: pf rules not loaded");
 	if ((altqsupport && (loadopt & (PFCTL_FLAG_ALTQ | PFCTL_FLAG_ALL)) != 0))
 		if (check_commit_altq(dev, opts) != 0)
-			errx(1, "errors in altq config");
+			ERRX("errors in altq config");
 	if ((opts & PF_OPT_NOACTION) == 0) {
 		if ((loadopt & (PFCTL_FLAG_NAT | PFCTL_FLAG_ALL)) != 0) {
 			pr[PF_RULESET_NAT].rule.action = PF_NAT;
 			if (ioctl(dev, DIOCCOMMITRULES, &pr[PF_RULESET_NAT]) &&
 			    (errno != EINVAL || pf.rule_nr))
-				err(1, "DIOCCOMMITRULES NAT");
+				ERR("DIOCCOMMITRULES NAT");
 			pr[PF_RULESET_RDR].rule.action = PF_RDR;
 			if (ioctl(dev, DIOCCOMMITRULES, &pr[PF_RULESET_RDR]) &&
 			    (errno != EINVAL || pf.rule_nr))
-				err(1, "DIOCCOMMITRULES RDR");
+				ERR("DIOCCOMMITRULES RDR");
 			pr[PF_RULESET_BINAT].rule.action = PF_BINAT;
 			if (ioctl(dev, DIOCCOMMITRULES, &pr[PF_RULESET_BINAT]) &&
 			    (errno != EINVAL || pf.rule_nr))
-				err(1, "DIOCCOMMITRULES BINAT");
+				ERR("DIOCCOMMITRULES BINAT");
 		}
 		if (((altqsupport && (loadopt &
 		    (PFCTL_FLAG_ALTQ | PFCTL_FLAG_ALL)) != 0)) &&
 		    ioctl(dev, DIOCCOMMITALTQS, &pa.ticket))
-			err(1, "DIOCCOMMITALTQS");
+			ERR("DIOCCOMMITALTQS");
 		if ((loadopt & (PFCTL_FLAG_FILTER | PFCTL_FLAG_ALL)) != 0) {
 			pr[PF_RULESET_SCRUB].rule.action = PF_SCRUB;
 			if (ioctl(dev, DIOCCOMMITRULES, &pr[PF_RULESET_SCRUB]) &&
 			    (errno != EINVAL || pf.rule_nr))
-				err(1, "DIOCCOMMITRULES SCRUB");
+				ERR("DIOCCOMMITRULES SCRUB");
 			pr[PF_RULESET_FILTER].rule.action = PF_PASS;
 			if (ioctl(dev, DIOCCOMMITRULES, &pr[PF_RULESET_FILTER]) &&
 			    (errno != EINVAL || pf.rule_nr))
-				err(1, "DIOCCOMMITRULES FILTER");
+				ERR("DIOCCOMMITRULES FILTER");
+		}
+		if (loadopt & (PFCTL_FLAG_TABLE | PFCTL_FLAG_ALL)) {
+			if (pfr_ina_commit(pf.tticket, NULL, NULL, 0))
+				ERR("commit table");
+			pf.tdirty = 0;
 		}
-		if (loadopt & (PFCTL_FLAG_TABLE | PFCTL_FLAG_ALL))
-			pfctl_commit_table();
 	}
 	if (fin != stdin)
 		fclose(fin);
@@ -1023,9 +1031,17 @@ pfctl_rules(int dev, char *filename, int opts, char *anchorname,
 	/* process "load anchor" directives */
 	if (!anchorname[0] && !rulesetname[0])
 		if (pfctl_load_anchors(dev, opts) == -1)
-			return (-1);
+			ERRX("load anchors");
 
 	return (0);
+
+_error:
+	if (pf.tdirty) /* cleanup kernel leftover */
+		pfr_ina_begin(NULL, NULL, 0);
+	exit(1);
+
+#undef ERR
+#undef ERRX
 }
 
 int
diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h
index faea7e1ccbc..48b5d7ab184 100644
--- a/sbin/pfctl/pfctl_parser.h
+++ b/sbin/pfctl/pfctl_parser.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_parser.h,v 1.62 2003/07/03 09:13:06 cedric Exp $ */
+/*	$OpenBSD: pfctl_parser.h,v 1.63 2003/07/03 21:09:13 cedric Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -60,6 +60,8 @@ struct pfctl {
 	int dev;
 	int opts;
 	int loadopt;
+	int tticket;			/* table ticket */
+	int tdirty;			/* kernel dirty */
 	u_int32_t rule_nr;
 	struct pfioc_pooladdr paddr;
 	struct pfioc_rule *prule[PF_RULESET_MAX];
@@ -154,10 +156,8 @@ void	 print_altq(const struct pf_altq *, unsigned, struct node_queue_bw *,
 void	 print_queue(const struct pf_altq *, unsigned, struct node_queue_bw *,
 	     int, struct node_queue_opt *);
 
-void	pfctl_begin_table(void);
 int	pfctl_define_table(char *, int, int, int, const char *, const char *,
-	    struct pfr_buffer *);
-void	pfctl_commit_table(void);
+	    struct pfr_buffer *, int);
 
 struct icmptypeent {
 	const char *name;
diff --git a/sbin/pfctl/pfctl_table.c b/sbin/pfctl/pfctl_table.c
index afd45a8c090..ebe9dfa340b 100644
--- a/sbin/pfctl/pfctl_table.c
+++ b/sbin/pfctl/pfctl_table.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_table.c,v 1.46 2003/07/03 09:13:06 cedric Exp $ */
+/*	$OpenBSD: pfctl_table.c,v 1.47 2003/07/03 21:09:13 cedric Exp $ */
 
 /*
  * Copyright (c) 2002 Cedric Berger
@@ -60,12 +60,8 @@ static int	load_addr(struct pfr_buffer *, int, char *[], char *, int);
 static void	print_addrx(struct pfr_addr *, struct pfr_addr *, int);
 static void	print_astats(struct pfr_astats *, int);
 static void	radix_perror(void);
-static void	inactive_cleanup(void);
 static void	xprintf(int, const char *, ...);
 
-static int	 ticket, inactive;
-extern char	*__progname;
-
 static const char	*stats_text[PFR_DIR_MAX][PFR_OP_TABLE_MAX] = {
 	{ "In/Block:",	"In/Pass:",	"In/XPass:" },
 	{ "Out/Block:",	"Out/Pass:",	"Out/XPass:" }
@@ -441,27 +437,14 @@ print_astats(struct pfr_astats *as, int dns)
 void
 radix_perror(void)
 {
+	extern char *__progname;
 	fprintf(stderr, "%s: %s.\n", __progname, pfr_strerror(errno));
 }
 
-void
-pfctl_begin_table(void)
-{
-	static int hookreg;
-
-	if (pfr_ina_begin(&ticket, NULL, 0) != 0) {
-		radix_perror();
-		exit(1);
-	}
-	if (!hookreg) {
-		atexit(inactive_cleanup);
-		hookreg = 1;
-	}
-}
-
 int
 pfctl_define_table(char *name, int flags, int addrs, int noaction,
-    const char *anchor, const char *ruleset, struct pfr_buffer *ab)
+    const char *anchor, const char *ruleset, struct pfr_buffer *ab,
+    int ticket)
 {
 	struct pfr_table tbl;
 	int rv = 0;
@@ -477,7 +460,6 @@ pfctl_define_table(char *name, int flags, int addrs, int noaction,
 			errx(1, "pfctl_define_table: strlcpy");
 		tbl.pfrt_flags = flags;
 
-		inactive = 1;
 		if (pfr_ina_define(&tbl, ab->pfrb_caddr, ab->pfrb_size, NULL,
 		    NULL, ticket, addrs ? PFR_FLAG_ADDRSTOO : 0) != 0) {
 			rv = -1;
@@ -488,23 +470,6 @@ pfctl_define_table(char *name, int flags, int addrs, int noaction,
 }
 
 void
-pfctl_commit_table(void)
-{
-	if (pfr_ina_commit(ticket, NULL, NULL, 0) != 0) {
-		radix_perror();
-		exit(1);
-	}
-	inactive = 0;
-}
-
-void
-inactive_cleanup(void)
-{
-	if (inactive)
-		pfr_ina_begin(NULL, NULL, 0);
-}
-
-void
 xprintf(int opts, const char *fmt, ...)
 {
 	va_list args;