On 28/03/17(Tue) 13:02, Alexandr Nedvedicky wrote: > [...] > > > > - s/test_status/action/ as it's done everywhere else? > > I've opted to test_status, because it's something different to 'action' > as we use it in current code.
I agree with you for test_status. What about naming the enum and use it instead of 'int' for the variable? This implicitly documents the possible option and allow the compiler to check for missing cases in switch. > > Does this pass pfctl regress tests? > > I'm about to run those tests for OpenBSD. Did you manage to do that? > > While I haven't noticed anything criminal here, it makes me > > wonder if it'd be possible to do this change in a few steps: > > factor out rule maching from pf_test_rule and then bring in > > anchor changes? > > > > if I understand you right, you basically want me to make change > in two steps: > > the first step splits current pf_test_rule() to pf_match_rule() and > pf_test_rule() > > the second step will kill global anchor stack array by introducing > a true recursion. The patch will remove pf_step_out_of_anchor() > function. > > I think I can do it. And also as Theo pointed out there is no rush > to get that patch to tree. Now *is* the time to commit the first step, the refactoring. Once that's done we can discuss the introduction of the context. Could you come up with such diff? Cheers, Martin
