Hi, 

Comments below,

> On 19/01/2019, at 2:32 PM, Klemens Nanni <[email protected]> wrote:
> 
> A few assorted nits for consistency and proper format, no object change.
> 
> OK?
> 
> Index: pfctl.c
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
> retrieving revision 1.365
> diff -u -p -r1.365 pfctl.c
> --- pfctl.c   11 Jan 2019 03:09:24 -0000      1.365
> +++ pfctl.c   19 Jan 2019 01:29:20 -0000
> @@ -1485,7 +1485,6 @@ pfctl_load_ruleset(struct pfctl *pf, cha
>                       }
>               } else if (pf->opts & PF_OPT_VERBOSE)
>                       printf("\n");
> -
>       }
> 
>       if (pf->optimize)
> @@ -1851,7 +1850,6 @@ pfctl_set_limit(struct pfctl *pf, const 
> {
>       int i;
> 
> -
>       for (i = 0; pf_limits[i].name; i++) {
>               if (strcasecmp(opt, pf_limits[i].name) == 0) {
>                       pf->limit[pf_limits[i].index] = limit;
> @@ -2217,7 +2215,7 @@ pfctl_show_anchors(int dev, int opts, ch
>                       err(1, "DIOCGETRULESET");
>               if (!strcmp(pr.name, PF_RESERVED_ANCHOR))
>                       continue;
> -             sub[0] = 0;
> +             sub[0] = '\0';
>               if (pr.path[0]) {
>                       strlcat(sub, pr.path, sizeof(sub));
>                       strlcat(sub, "/", sizeof(sub));
> @@ -2235,6 +2233,7 @@ const char *
> pfctl_lookup_option(char *cmd, const char **list)
> {
>       const char *item = NULL;
> +
>       if (cmd != NULL && *cmd)
>               for (; *list; list++)
>                       if (!strncmp(cmd, *list, strlen(cmd))) {
> @@ -2580,15 +2579,15 @@ main(int argc, char *argv[])
>                       opts |= PF_OPT_SHOWALL;
>                       pfctl_load_fingerprints(dev, opts);
> 
> -                     pfctl_show_rules(dev, path, opts, 0, anchorname,
> -                         0, 0, -1);
> +                     pfctl_show_rules(dev, path, opts, PFCTL_SHOW_RULES,
> +                         anchorname, 0, 0, -1);
>                       pfctl_show_queues(dev, ifaceopt, opts,
>                           opts & PF_OPT_VERBOSE2);
>                       pfctl_show_states(dev, ifaceopt, opts, -1);
>                       pfctl_show_src_nodes(dev, opts);
>                       pfctl_show_status(dev, opts);
> -                     pfctl_show_rules(dev, path, opts, 1, anchorname,
> -                         0, 0, -1);
> +                     pfctl_show_rules(dev, path, opts, PFCTL_SHOW_LABELS,
> +                         anchorname, 0, 0, -1);
>                       pfctl_show_timeouts(dev, opts);
>                       pfctl_show_limits(dev, opts);
>                       pfctl_show_tables(anchorname, opts);
> @@ -2671,7 +2670,7 @@ main(int argc, char *argv[])
>       if (optiopt != NULL) {
>               switch (*optiopt) {
>               case 'n':
> -                     optimize = 0;
> +                     optimize = PF_OPTIMIZE_NONE;
>                       break;
>               case 'b':
>                       optimize |= PF_OPTIMIZE_BASIC;
> Index: pfctl_parser.h
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl_parser.h,v
> retrieving revision 1.112
> diff -u -p -r1.112 pfctl_parser.h
> --- pfctl_parser.h    6 Sep 2018 15:07:34 -0000       1.112
> +++ pfctl_parser.h    19 Jan 2019 01:13:13 -0000
> @@ -57,6 +57,7 @@
> #define PF_NAT_PROXY_PORT_LOW 50001
> #define PF_NAT_PROXY_PORT_HIGH        65535
> 
> +#define PF_OPTIMIZE_NONE     0x0000

these PF_OPTIMIZE_* are bit-field definitions,
see e.g. pfctl_optimize.c:299. 

But PF_OPTIMIZE_NONE is not, as pf->optimize & PF_OPTIMIZE_NONE 
is never true, and pf->optimize |= PF_OPTIMIZE_NONE has no effect. 

so I would leave this as optimize = 0; and drop PF_OPTIMIZE_NONE.

otherwise, ok procter

best, 
Richard. 


> #define PF_OPTIMIZE_BASIC     0x0001
> #define PF_OPTIMIZE_PROFILE   0x0002
> 
> 

Reply via email to