Henning Brauer <lists-openbsdtech <at> bsws.de> writes:
> > #if NVLAN > 0
> > if (ifp->if_type == IFT_L2VLAN)
> > return vlan_encap(ifp, m);
> > #endif
>
> I don't think so, really. We're talking about 25 lines of code, in a
> function that either prepends a regular ethernet header or a
> vlan-ethernet header. Having both cases next to each other makes it
> much more obvious what's going on imho.
If I would be writing such a code, I wouldn't be breaking Single
Responsibility Pattern and obfuscate ether_addheader with VLAN
logic, but add small dedicated methods instead.
Something like:
...
#if NVLAN > 0
//void ether_addvlanhdr1...
//void ether_addvlanhdr2...
#endif
...
#if NVLAN > 0
if (ifp->if_type == IFT_L2VLAN) {
if ((p->if_capabilities & IFCAP_VLAN_HWTAGGING) &&
(ifv->ifv_type == ETHERTYPE_VLAN)) {
//ether_addvlanhdr1();
} else {
//ether_addvlanhdr2();
return (0);
}
}
#endif
if (ether_addheader(&m, etype, esrc, edst) == -1)
senderr(ENOBUFS);
