Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-06 Thread Klemens Nanni
On Sat, Apr 06, 2019 at 02:37:05AM +0200, Alexandr Nedvedicky wrote:
> updated diff is attached. I'll commit the change after unlock.
OK kn with comments inline.

> + pf.ifname = strdup("none");
> + if (pf.ifname == NULL)
> + warn("%s: Warning: can't reset loginterface\n", __func__);
> + else
> + pf.ifname_set = 1;

We should fail hard as in almost all other strdup(3) use cases.
Failure means the system ran out of memory, there's no point in going
any further.

So just something like

pf.ifname = strdup("none");
if (pf.ifname == NULL)
err(1, "%s: strdup", __func__);

> + if (pfctl_trans(dev, , DIOCXBEGIN, 0))
> + warn("%s, DIOCXBEGIN", __func__);
Turn this comma into a double colon.

> + if (pfctl_trans(dev, , DIOCXCOMMIT, 0))
> + warn("%s, DIOCXCOMMIT", __func__);
Same here.



Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-06 Thread Gleydson Soares
 +void
> +pfctl_reset(int dev, int opts)
> +{
> + struct pfctlpf;
> + struct pfr_buffer t;
> + int i;
> +
> + pf.dev = dev;
> + pfctl_init_options();
> +
> + /* Force reset upon pfctl_load_options() */
> + pf.debug_set = 1;
> + pf.reass_set = 1;
> + pf.syncookieswat_set = 1;
> + pf.ifname = strdup("none");
> + if (pf.ifname == NULL)
> + warn("%s: Warning: can't reset loginterface\n", __func__);

do you really need this
extra newline here?
warn() itself already includes
one.
> + else
> + pf.ifname_set = 1;
> +
> + memset(, 0, sizeof(t));
> + t.pfrb_type = PFRB_TRANS;
> + if (pfctl_trans(dev, , DIOCXBEGIN, 0))
> + warn("%s, DIOCXBEGIN", __func__);
> +
> + for (i = 0; pf_limits[i].name; i++)
> + pf.limit_set[pf_limits[i].index] = 1;
> +
> + for (i = 0; pf_timeouts[i].name; i++)
> + pf.timeout_set[pf_timeouts[i].timeout] = 1;
> +
> + pfctl_load_options();
> +
> + if (pfctl_trans(dev, , DIOCXCOMMIT, 0))
> + warn("%s, DIOCXCOMMIT", __func__);
> +
> + pfctl_clear_interface_flags(dev, opts);
> +}