Hello, On Tue, Dec 26, 2017 at 10:02:00PM +0100, Alexander Bluhm wrote: > Hi, > > The functions to link the pf state keys always confused me a bit. > I think the new naming and code of the functions is more consistent. > > ok?
I like your change. You have my OK with small change here: </snip> > Index: net/pf.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v > retrieving revision 1.1051 > diff -u -p -r1.1051 pf.c > --- net/pf.c 24 Dec 2017 14:18:19 -0000 1.1051 > +++ net/pf.c 26 Dec 2017 20:52:25 -0000 </snip> > void > -pf_pkt_state_key_ref(struct mbuf *m) > +pf_mbuf_link_state_key(struct mbuf *m, struct pf_state_key *sk) > { > - pf_state_key_ref(m->m_pkthdr.pf.statekey); KASSERT(m->m_pkthdr.pf.statekey == NULL); > + m->m_pkthdr.pf.statekey = pf_state_key_ref(sk); > } I think we should have KASSERT() above as kind of check if more safety belts are needed to mitigate potential memory leak. thanks and regards sasha