ipfw: use typed pointers to access network protocols headers where possible

[glebius]

Summary

The 'void *ulp' is still in action, but where possible prefer a typed
pointer. Get rid of associated pre-processor macros.

[glebius]

Treat this as a suggestion. IMHO, this is more readable than the old version. If everybody disagrees with me, I'm fine with abandoning this.

[bz]

The diff may not look great but I like the spelling out of types rather than hiding casts behind macros.

I was going to say I'd start the block under the case and not at the end of the line but it seems the more dominant style indeed is what you chose, so ignore this one.

[lytboris_gmail.com]

sys/netpfil/ipfw/ip_fw2.c

1590

We have insntoc/insntod macros with similar goal, but for ipfw insn structure.

Not sure if we need a stack variables scattered across the code instead of something similar to ulptod(ulp, icmp6).

Yep, this sounds much like reformatting ICMP6(ulp) et al. into a flexible form.

[glebius]

sys/netpfil/ipfw/ip_fw2.c

1590

IMHO, all these macros that hide C basics are beneficial when you work with a single project for a long time and both your eyes and fingers have the muscle memory to immediately recognize them and type them. The downside is that the code is not very welcoming to a newcomer. The struct ip, struct tcp, struct icmp6_hdr and etc are well known to any network stack programmer and adoption is even wider than just FreeBSD. So, for a newcomer the suggested code is much more readable without jumping and checking wtf is ICMP6() or ulptod(). Even a returning co-author after years of working in different areas of the network stack will stumble upon this micro-obfuscations.

[lytboris_gmail.com]

sys/netpfil/ipfw/ip_fw2.c

1590

A perfectly valid point indeed. But we're talking about hot path and on-stack variables may require an additional assignment in contrast to type casting.

[glebius]

sys/netpfil/ipfw/ip_fw2.c

1590

Compiler will produce same code for both variants. Maybe only with -O0 could be different.