Re: CVS commit: src/sys/netinet

[Martin Husemann <martin%duskware.de@localhost>]

On Thu, Apr 14, 2011 at 07:03:49AM +0000, YAMAMOTO Takashi wrote:
> please just weaken the assertion and clear the flag,
> rather than complicating the code.

I'm not quite sure I see exactly what you would like the code to look like.
What we have now:

        /*
         * We may not use checksums on loopback interfaces
         */
        if (__predict_false(ifp == NULL) ||
            IN_NEED_CHECKSUM(ifp, M_CSUM_IPv4)) {
                if (sw_csum & M_CSUM_IPv4) {
                        ip->ip_sum = in_cksum(m, hlen);
                        m->m_pkthdr.csum_flags &= ~M_CSUM_IPv4;
                } else {
                        KASSERT(m->m_pkthdr.csum_flags & M_CSUM_IPv4);
                        KASSERT(M_CSUM_DATA_IPv4_IPHL(m->m_pkthdr.csum_data) >=
                                sizeof(struct ip));
                }
        }

This could be refactored into:

        if (sw_csum & M_CSUM_IPv4) {
                ip->ip_sum = in_cksum(m, hlen);
                m->m_pkthdr.csum_flags &= ~M_CSUM_IPv4;
        } else if (__predict_false(ifp == NULL)
                    || IN_NEED_CHECKSUM(ifp, M_CSUM_IPv4)) {
                        KASSERT(m->m_pkthdr.csum_flags & M_CSUM_IPv4);
                        KASSERT(M_CSUM_DATA_IPv4_IPHL(m->m_pkthdr.csum_data) >=
                                sizeof(struct ip));
                }
        }

or a variant that embeds the else if condition into both KASSERTs, which seems
pretty ugly to me.

Can you please clarify?

Thanks,

Martin