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

Reply via email to