On Tue, Sep 26, 2017 at 11:37 +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> whenever administrator asks pfctl to modify/remove anchor, which does not
> exist, the pfctl(8) prints warning 'pfctl: DIOCGETRULES: Invalid argument'.
> Few users on Solaris wants pfctl(8) to be more helpful.
>
> The 'Invalid Argument' (EINVAL) is returned when particular anchor is not
> found. Patch below changes pfctl output to this:
>
> # pfctl -sA
> # pfctl -a Foo -sr
> Anchor 'Foo' not found.
> #
>
> OK?
>
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");
uipc_mbuf.c are clearly unrelated, but what about the chunk below?
> @@ -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.
> 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 11:36:50 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)
> + err(1, "pfctl_clear_rules");
> + else
> + errx(1, "Anchor '%s' not found.\n", anchorname);
> + }
> if ((opts & PF_OPT_QUIET) == 0)
> fprintf(stderr, "rules cleared\n");
> }
> @@ -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';
> }
>
> @@ -871,7 +881,11 @@ 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)
> + warn("DIOCGETRULES");
> + else
> + fprintf(stderr, "Anchor '%s' not found.\n",
> + anchorname);
> ret = -1;
> goto error;
> }
> @@ -886,7 +900,10 @@ pfctl_show_rules(int dev, char *path, in
>
> pr.rule.action = PF_PASS;
> if (ioctl(dev, DIOCGETRULES, &pr)) {
> - warn("DIOCGETRULES");
> + if (errno != EINVAL)
> + warn("DIOCGETRULES");
> + else
> + fprintf(stderr, "Anchor '%s' not found.\n", anchorname);
> ret = -1;
> goto error;
> }
> diff -r 215db23c6b05 src/sys/kern/uipc_mbuf.c
> --- src/sys/kern/uipc_mbuf.c Mon Sep 25 13:38:48 2017 +0200
> +++ src/sys/kern/uipc_mbuf.c Tue Sep 26 11:36:50 2017 +0200
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: uipc_mbuf.c,v 1.249 2017/09/15 18:13:05 bluhm Exp $ */
> +/* $OpenBSD: uipc_mbuf.c,v 1.248 2017/05/27 16:41:10 bluhm Exp $ */
> /* $NetBSD: uipc_mbuf.c,v 1.15.4.1 1996/06/13 17:11:44 cgd Exp $ */
>
> /*
> @@ -804,13 +804,12 @@ m_adj(struct mbuf *mp, int req_len)
> struct mbuf *m;
> int count;
>
> - if (mp == NULL)
> + if ((m = mp) == NULL)
> return;
> if (len >= 0) {
> /*
> * Trim from head.
> */
> - m = mp;
> while (m != NULL && len > 0) {
> if (m->m_len <= len) {
> len -= m->m_len;
> @@ -834,7 +833,6 @@ m_adj(struct mbuf *mp, int req_len)
> */
> len = -len;
> count = 0;
> - m = mp;
> for (;;) {
> count += m->m_len;
> if (m->m_next == NULL)
> @@ -855,16 +853,15 @@ m_adj(struct mbuf *mp, int req_len)
> * Find the mbuf with last data, adjust its length,
> * and toss data from remaining mbufs on chain.
> */
> - if (mp->m_flags & M_PKTHDR)
> - mp->m_pkthdr.len = count;
> m = mp;
> - for (;;) {
> + if (m->m_flags & M_PKTHDR)
> + m->m_pkthdr.len = count;
> + for (; m; m = m->m_next) {
> if (m->m_len >= count) {
> m->m_len = count;
> break;
> }
> count -= m->m_len;
> - m = m->m_next;
> }
> while ((m = m->m_next) != NULL)
> m->m_len = 0;
> --------8<---------------8<---------------8<------------------8<--------
>
