Hello Mike,
thanks for looking at it.
>
> Can you please reverse the check so that adding conditions is possible
> and the default is in the 'else' branch, i.e.
>
> if (errno == EINVAL)
> errx(1, "Anchor '%s' not found.\n", anchorname);
> else
> err(1, "pfctl_clear_rules");
good point, makes sense
>
> uipc_mbuf.c are clearly unrelated, but what about the chunk below?
yes uipc_mbuf.c is clear martian (I've forgot to update my branch).
>
> > @@ -850,7 +860,7 @@ pfctl_show_rules(int dev, char *path, in
> > * to the kernel.
> > */
> > if ((p = strrchr(anchorname, '/')) != NULL &&
> > - p[1] == '*' && p[2] == '\0') {
> > + ((p[1] == '*' && p[2] == '\0') || (p[1] == '\0'))) {
> > p[0] = '\0';
> > }
> >
>
> I think this requires an explanation.
The change above belongs to other patch I hope to send. The other patch
sanitizes anchor name. For example names:
Foo/*
Foo/
will be treated as Foo consistently. Let's save the topic for another
email thread.
below is updated patch. The list of changes is as follows:
- removed uipc_mbuf.c changes
- removed anchorname sanitization at pfctl_show_rules()
- inverting the if (... = EINVAL) test as suggested by Mike
- using warnx() in favor of fprintf(stderr, ...), it makes my
change consistent with existing code.
thanks and
regards
sasha
--------8<---------------8<---------------8<------------------8<--------
diff -r 215db23c6b05 src/sbin/pfctl/pfctl.c
--- src/sbin/pfctl/pfctl.c Mon Sep 25 13:38:48 2017 +0200
+++ src/sbin/pfctl/pfctl.c Tue Sep 26 13:01:48 2017 +0200
@@ -318,13 +318,23 @@ void
pfctl_clear_rules(int dev, int opts, char *anchorname)
{
struct pfr_buffer t;
+ char *p;
+
+ p = strrchr(anchorname, '/');
+ if (p != NULL && p[1] == '\0')
+ errx(1, "%s: bad anchor name %s", __func__, anchorname);
memset(&t, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
+
if (pfctl_add_trans(&t, PF_TRANS_RULESET, anchorname) ||
pfctl_trans(dev, &t, DIOCXBEGIN, 0) ||
- pfctl_trans(dev, &t, DIOCXCOMMIT, 0))
- err(1, "pfctl_clear_rules");
+ pfctl_trans(dev, &t, DIOCXCOMMIT, 0)) {
+ if (errno == EINVAL)
+ errx(1, "Anchor '%s' not found.", anchorname);
+ else
+ err(1, "pfctl_clear_rules");
+ }
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "rules cleared\n");
}
@@ -871,7 +881,10 @@ pfctl_show_rules(int dev, char *path, in
if (opts & PF_OPT_SHOWALL) {
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, &pr)) {
- warn("DIOCGETRULES");
+ if (errno == EINVAL)
+ warnx("Anchor '%s' not found.", anchorname);
+ else
+ warn("DIOCGETRULES");
ret = -1;
goto error;
}
@@ -886,7 +899,10 @@ pfctl_show_rules(int dev, char *path, in
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, &pr)) {
- warn("DIOCGETRULES");
+ if (errno == EINVAL)
+ warnx("Anchor '%s' not found.", anchorname);
+ else
+ warn("DIOCGETRULES");
ret = -1;
goto error;
}
--------8<---------------8<---------------8<------------------8<--------
