Sponsored by: Rubicon Communications, LLC ("Netgate")
Details
Details
- Reviewers
imp - Group Reviewers
network pfsense - Commits
- rG959af5a89b20: sys: make callout.h self-contained
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
You'll need to remove these from badfiles.inc too...
also we don't test the _*.h files.
| sys/sys/_callout.h | ||
|---|---|---|
| 43 | Does _types.h suffice? | |
Comment Actions
That's in D33506
also we don't test the _*.h files.
Not directly, but this breaks callout.h (and a few headers that include callout.h). It seems appropriate to add the include to the header that actually uses the type directly, rather than to say callout.h, which includes this header.
| sys/sys/_callout.h | ||
|---|---|---|
| 43 | Unfortunately not. _callout.h uses sbintime_t, which is typedef'd in sys/types.h itself. | |
| sys/sys/_callout.h | ||
|---|---|---|
| 43 | OK. I'll have to take a closer look at where this is used to see what to do here. Most of the _xxx.h are supposed to, in theory, avoid including the non _ version where both exist (eg here with types.h and _types.h). types.h may be a special case, though and I need to look at it more closely. | |
Comment Actions
I'd be tempted to make all of sys/callout.h #ifdef _KERNEL rather than the subset it is today. It's a kernel only interface that none of the kernel memory groveling programs reference.
I'd be tempted to do similar to sys/taskqueue.h for the same reason.
However, having said all that, sys/callout.h is included in sys/proc.h to get struct callout which lives inside of struct thread, so we can't do the above at least for sys/callout.h. So it's more than just the #defines that need to be visible (though maybe they don't need to be).
Based on that, I'd be tempted to add a typedef int64_t sbintime_t; to _types.h, change the typedef in types.h to be typedef sbintime_t sbintime_t and then change _callout.h to use sbintime_t. It might also be possible to change sys/callout.h to sys/_callout.h in sys/proc.h, but that likely would have more ripple effect potential based on the current namespace pollution in the kernel so it may not be worth making that change.
I suspect that will have less fallout than this change with less namespace pollution which pops up in weird places...
So I'd prefer that things be fixed that way. I suspect we'll need __sbintime_t before this is all over more urgently than for this example and it would make this work with no additional namespace pollution (though one could legitimately have a debate over this specific case).