On 29/10/15(Thu) 02:49, Mike Belopuhov wrote: > On 28 October 2015 at 18:41, Alexandr Nedvedicky > <[email protected]> wrote: > > Hello Mike, > > > > just a quick question: > > > > are you going to commit your pfi_kif_find() et. al.? > > or more work is needed there? > > > > I need OKs > [...] > > pfsync_in_clr(caddr_t buf, int len, int count, int flags) > > { > > struct pfsync_clr *clr; > > - int i; > > - > > struct pf_state *st, *nexts; > > - struct pf_state_key *sk, *nextsk; > > - struct pf_state_item *si; > > + struct pfi_kif *kif = NULL; > > u_int32_t creatorid; > > + int i; > > > > for (i = 0; i < count; i++) { > > clr = (struct pfsync_clr *)buf + len * i; > > creatorid = clr->creatorid; > > + if (strlen(clr->ifname) && > > + (kif = pfi_kif_find(clr->ifname)) == NULL) > > + continue; > > -[...] > > + for (st = RB_MIN(pf_state_tree_id, &tree_id); st; st = > > nexts) { > > + nexts = RB_NEXT(pf_state_tree_id, &tree_id, st); > > + if (st->creatorid == creatorid && > > + ((kif && st->kif == kif) || !kif)) {
I find this check confusing. Since you're continuing when "kif == NULL" can't you change this check into "st->kif == kif"? Or is it possible for strlen(clr->ifname) to return 0 in which case you might end up comparing a different ``kif''? > > + SET(st->state_flags, PFSTATE_NOSYNC); > > + pf_unlink_state(st); > > } > > } > > }
