Include opt_inet.h and opt_inet6.h early in the files including virtio_net.h, since they use INET and/or INET6. In addition to that, also include them in virtio_net.h, since this files contains inline function definitions using INET and INET6.
While there, remove redundant inclusion of sys/types.h, since it is included already by sys/param.h.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
if_ptnet.c and if_vtnet.c should include these files, since they use INET and/or INET6. They should not rely on other include files like virto_net.h including opt_inet.h and opt_inet6.h for them. If files include opt_*.h, these includes should be very early. So I guess the changes to if_ptnet.c and if_vtnet.c are appropriate.
A valid question is if virto_net.h should also include opt_inet.h and opt_inet6.h. I don't know. I will bring that up in tomorrow's transport call.
Comment Actions
A valid question is if virto_net.h should also include opt_inet.h and opt_inet6.h. I don't know. I will bring that up in tomorrow's transport call.
Normally, we avoid that. But also normally we avoid inlines. We avoid including opt_*.h in .h files because those tend to become user-visible in time. And there's been some absuse of that to drive ABI differences (mostly away from FreeBSD) and so there's a big desire to disallow that. Plus, .h files tend to become user visible, and the opt files aren't around for user-land builds.
But in this case, it's purely internal, *AND* virtio_net.h uses the defies. I think it would be fine to include them there.. I'd do it there as well, as a backup, because it uses it. I'd still move the includes because these files also test those defines and the general rule is to put opt_*.h includes before everything else and you don't want these files to assume that virtio_net.h to snag it for them.
tl;dr: this is a weird case, and I'd do both the move and add it to the .h.
| sys/dev/netmap/if_ptnet.c | ||
|---|---|---|
| 29–31 | A nickle says this include can now be removed. The opt includes should be before it anyway. | |
Comment Actions
As Warner suggested: don't include sys/param.h and sys/types.h, since sys/param.h already includes sys/types.h as mentioned in style.9.
Comment Actions
As discussed on today's FreeBSD transport meeting, don't include opt_inet.h and opt_inet6.h in virtio_net.h.
Gleb had the idea to extend the opt_*.h machinery in a way that every file needing these includes can check for them. virtio_net.h will be the first consumer of this. But that will be a separate commit.