On Mon, Jun 12, 2017 at 01:59:07PM +0200, Mike Belopuhov wrote: > On Sun, Jun 11, 2017 at 15:03 +0100, Raymond wrote: > > Transform the following functions (which never return anything other than > > 0, and whose return value is never used) to void: > > > > * pfctl_clear_stats, pfctl_clear_interface_flags, pfctl_clear_rules, > > pfctl_clear_src_nodes, pfctl_clear_states > > * pfctl_kill_src_nodes, pfctl_net_kill_states, pfctl_label_kill_states, > > pfctl_id_kill_states, pfctl_key_kill_states > > > > inside main: merge two identical if conditions next to each other into one. > > > > credit to > > - awolk@ for the code reading > > - mikeb for pointing out we can void all _clear_ functions > > - ghostyy for pointing out all _kill_ functions can be voided > > > > Looks good to me. I was going to point out that pfctl_clear_tables > should also be converted, but leave that for a rainy day since some > extra return value checking of pfctl_table call is probably in order. >
It was a rainy evening here, so here's the updated pfctl diff. Note that it's based on top of the original diff from this thread. The additionally modified files are pfctl_table.c and pfctl.h. Changes: voided: - pfctl_clear_tables - pfctl_show_tables - pfctl_show_ifaces Tested on amd64 -current with a kernel modified to output the following errors for the matching ioctls: - DIOCRCLRTABLES -> ESRCH - DIOCRGETTABLES -> ESRCH - DIOCIGETIFACES -> ENOENT Attaching the pf_ioctl.diff just for reference. Looking for feedback, and OK's. # show interfaces $ doas pfctl -sI pfctl: Anchor or Ruleset does not exist. $ echo $? 0 # show tables $ doas pfctl -sT pfctl: Table does not exist. $ echo $? 0 # clear tables $ doas pfctl -F Tables pfctl: Table does not exist. $ echo $? 0 # show all $ doas pfctl -sa ... trimmed output ... pfctl: Table does not exist. ... trimmed output ... $ echo $? 0 Behavior of the modified pfctl binary # show interfaces $ doas ./pfctl -sI pfctl: pfctl_show_ifaces: Anchor or Ruleset does not exist $ echo $? 1 # show tables $ doas ./pfctl -sT pfctl: Table does not exist. pfctl: pfctl_show_tables: Table does not exist $ echo $? 1 # clear tables $ doas ./pfctl -F Tables pfctl: Table does not exist. pfctl: pfctl_show_tables: Table does not exist $ echo $? 1 # show all $ doas ./pfctl -sa pfctl: Table does not exist. pfctl: pfctl_show_tables: Table does not exist $ echo $? 1
? openbsd-daily-pfctl-1.diff ? parse.c ? pfctl ? rain.diff Index: pfctl.c =================================================================== RCS file: /cvs/src/sbin/pfctl/pfctl.c,v retrieving revision 1.344 diff -u -p -r1.344 pfctl.c --- pfctl.c 30 May 2017 12:13:04 -0000 1.344 +++ pfctl.c 12 Jun 2017 19:14:27 -0000 @@ -61,17 +61,17 @@ void usage(void); int pfctl_enable(int, int); int pfctl_disable(int, int); void pfctl_clear_queues(struct pf_qihead *); -int pfctl_clear_stats(int, const char *, int); -int pfctl_clear_interface_flags(int, int); -int pfctl_clear_rules(int, int, char *); -int pfctl_clear_src_nodes(int, int); -int pfctl_clear_states(int, const char *, int); +void pfctl_clear_stats(int, const char *, int); +void pfctl_clear_interface_flags(int, int); +void pfctl_clear_rules(int, int, char *); +void pfctl_clear_src_nodes(int, int); +void pfctl_clear_states(int, const char *, int); void pfctl_addrprefix(char *, struct pf_addr *); -int pfctl_kill_src_nodes(int, const char *, int); -int pfctl_net_kill_states(int, const char *, int, int); -int pfctl_label_kill_states(int, const char *, int, int); -int pfctl_id_kill_states(int, int); -int pfctl_key_kill_states(int, const char *, int, int); +void pfctl_kill_src_nodes(int, const char *, int); +void pfctl_net_kill_states(int, const char *, int, int); +void pfctl_label_kill_states(int, const char *, int, int); +void pfctl_id_kill_states(int, int); +void pfctl_key_kill_states(int, const char *, int, int); int pfctl_parse_host(char *, struct pf_rule_addr *); void pfctl_init_options(struct pfctl *); int pfctl_load_options(struct pfctl *); @@ -278,7 +278,7 @@ pfctl_disable(int dev, int opts) return (0); } -int +void pfctl_clear_stats(int dev, const char *iface, int opts) { struct pfioc_iface pi; @@ -296,10 +296,9 @@ pfctl_clear_stats(int dev, const char *i fprintf(stderr, " for interface %s", iface); fprintf(stderr, "\n"); } - return (0); } -int +void pfctl_clear_interface_flags(int dev, int opts) { struct pfioc_iface pi; @@ -313,10 +312,9 @@ pfctl_clear_interface_flags(int dev, int if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "pf: interface flags reset\n"); } - return (0); } -int +void pfctl_clear_rules(int dev, int opts, char *anchorname) { struct pfr_buffer t; @@ -329,20 +327,18 @@ pfctl_clear_rules(int dev, int opts, cha err(1, "pfctl_clear_rules"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "rules cleared\n"); - return (0); } -int +void pfctl_clear_src_nodes(int dev, int opts) { if (ioctl(dev, DIOCCLRSRCNODES)) err(1, "DIOCCLRSRCNODES"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "source tracking entries cleared\n"); - return (0); } -int +void pfctl_clear_states(int dev, const char *iface, int opts) { struct pfioc_state_kill psk; @@ -356,7 +352,6 @@ pfctl_clear_states(int dev, const char * err(1, "DIOCCLRSTATES"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "%d states cleared\n", psk.psk_killed); - return (0); } void @@ -409,7 +404,7 @@ pfctl_addrprefix(char *addr, struct pf_a freeaddrinfo(res); } -int +void pfctl_kill_src_nodes(int dev, const char *iface, int opts) { struct pfioc_src_node_kill psnk; @@ -509,10 +504,9 @@ pfctl_kill_src_nodes(int dev, const char if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d src nodes from %d sources and %d " "destinations\n", killed, sources, dests); - return (0); } -int +void pfctl_net_kill_states(int dev, const char *iface, int opts, int rdomain) { struct pfioc_state_kill psk; @@ -617,10 +611,9 @@ pfctl_net_kill_states(int dev, const cha if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d states from %d sources and %d " "destinations\n", killed, sources, dests); - return (0); } -int +void pfctl_label_kill_states(int dev, const char *iface, int opts, int rdomain) { struct pfioc_state_kill psk; @@ -645,11 +638,9 @@ pfctl_label_kill_states(int dev, const c if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d states\n", psk.psk_killed); - - return (0); } -int +void pfctl_id_kill_states(int dev, int opts) { struct pfioc_state_kill psk; @@ -680,11 +671,9 @@ pfctl_id_kill_states(int dev, int opts) if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d states\n", psk.psk_killed); - - return (0); } -int +void pfctl_key_kill_states(int dev, const char *iface, int opts, int rdomain) { struct pfioc_state_kill psk; @@ -741,8 +730,6 @@ pfctl_key_kill_states(int dev, const cha if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d states\n", psk.psk_killed); - - return (0); } int @@ -2558,13 +2545,11 @@ main(int argc, char *argv[]) } } - if ((rulesopt != NULL) && !anchorname[0]) - if (pfctl_clear_interface_flags(dev, opts | PF_OPT_QUIET)) - error = 1; - - if (rulesopt != NULL && !anchorname[0]) + if (rulesopt != NULL && !anchorname[0]) { + pfctl_clear_interface_flags(dev, opts | PF_OPT_QUIET); if (pfctl_file_fingerprints(dev, opts, PF_OSFP_FILE)) error = 1; + } if (rulesopt != NULL) { if (anchorname[0] == '_' || strstr(anchorname, "/_") != NULL) Index: pfctl.h =================================================================== RCS file: /cvs/src/sbin/pfctl/pfctl.h,v retrieving revision 1.53 diff -u -p -r1.53 pfctl.h --- pfctl.h 19 Jan 2015 23:52:02 -0000 1.53 +++ pfctl.h 12 Jun 2017 19:14:27 -0000 @@ -75,12 +75,12 @@ int pfi_get_ifaces(const char *, struct int pfi_clr_istats(const char *, int *, int); void pfctl_print_title(char *); -int pfctl_clear_tables(const char *, int); -int pfctl_show_tables(const char *, int); +void pfctl_clear_tables(const char *, int); +void pfctl_show_tables(const char *, int); int pfctl_command_tables(int, char *[], char *, const char *, char *, const char *, int); void warn_namespace_collision(const char *); -int pfctl_show_ifaces(const char *, int); +void pfctl_show_ifaces(const char *, int); FILE *pfctl_fopen(const char *, const char *); /* Index: pfctl_table.c =================================================================== RCS file: /cvs/src/sbin/pfctl/pfctl_table.c,v retrieving revision 1.75 diff -u -p -r1.75 pfctl_table.c --- pfctl_table.c 13 Apr 2017 07:30:21 -0000 1.75 +++ pfctl_table.c 12 Jun 2017 19:14:27 -0000 @@ -102,16 +102,18 @@ static const char *istats_text[2][2][2] table.pfrt_flags &= ~PFR_TFLAG_PERSIST; \ } while(0) -int +void pfctl_clear_tables(const char *anchor, int opts) { - return pfctl_table(0, NULL, NULL, "-F", NULL, anchor, opts); + if (pfctl_table(0, NULL, NULL, "-F", NULL, anchor, opts) == -1) + errx(1, "pfctl_clear_tables: %s", pfr_strerror(errno)); } -int +void pfctl_show_tables(const char *anchor, int opts) { - return pfctl_table(0, NULL, NULL, "-s", NULL, anchor, opts); + if (pfctl_table(0, NULL, NULL, "-s", NULL, anchor, opts) == -1) + errx(1, "pfctl_show_tables: %s", pfr_strerror(errno)); } int @@ -594,7 +596,7 @@ xprintf(int opts, const char *fmt, ...) /* interface stuff */ -int +void pfctl_show_ifaces(const char *filter, int opts) { struct pfr_buffer b; @@ -606,10 +608,8 @@ pfctl_show_ifaces(const char *filter, in for (;;) { pfr_buf_grow(&b, b.pfrb_size); b.pfrb_size = b.pfrb_msize; - if (pfi_get_ifaces(filter, b.pfrb_caddr, &b.pfrb_size)) { - radix_perror(); - return (1); - } + if (pfi_get_ifaces(filter, b.pfrb_caddr, &b.pfrb_size)) + errx(1, "pfctl_show_ifaces: %s", pfr_strerror(errno)); if (b.pfrb_size <= b.pfrb_msize) break; i++; @@ -618,7 +618,6 @@ pfctl_show_ifaces(const char *filter, in pfctl_print_title("INTERFACES:"); PFRB_FOREACH(p, &b) print_iface(p, opts); - return (0); } void
Index: pf_ioctl.c =================================================================== RCS file: /cvs/src/sys/net/pf_ioctl.c,v retrieving revision 1.315 diff -u -p -r1.315 pf_ioctl.c --- pf_ioctl.c 5 Jun 2017 22:18:28 -0000 1.315 +++ pf_ioctl.c 12 Jun 2017 20:49:37 -0000 @@ -1866,6 +1866,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a } error = pfr_clr_tables(&io->pfrio_table, &io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL); + error = ESRCH; break; } @@ -1902,6 +1903,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a } error = pfr_get_tables(&io->pfrio_table, io->pfrio_buffer, &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL); + error = ESRCH; break; } @@ -2412,6 +2414,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a } error = pfi_get_ifaces(io->pfiio_name, io->pfiio_buffer, &io->pfiio_size); + error = ENOENT; break; }