On 08/19/2022 11:24 AM Scott Cheloha <scottchel...@gmail.com> wrote:
On Aug 19, 2022, at 01:52, Claudio Jeker <cje...@diehard.n-r-g.com>
wrote:
On Thu, Aug 18, 2022 at 10:32:36PM -0500, Scott Cheloha wrote:
Hi,
clockframe is sometimes defined in cpu.h, sometimes in
frame.h, and sometimes defined once each in both header
files.
Can we put the clockframe definitions in frame.h? Always? It
is, at least ostensibly, a "frame".
I do not want to consolidate the clockframe definitions in
cpu.h because this is creating a circular dependency problem
for my clock interrupt patch.
In particular, cpu.h needs a data structure defined in a new
header file to add it to struct cpu_info on all
architectures, like this:
/* cpu.h */
#include <sys/clockintr.h>
struct cpu_info { /* ... */ struct clockintr_state; };
... but the header clockintr.h needs the clockframe
definition so it can prototype functions accepting a
clockframe pointer, like this:
/* clockintr.h */
#include <machine/frame.h> /* this works fine */
#ifdef this_does_not_work #include <machine/cpu.h> #endif
int clockintr_foo(struct clockframe *, int, short); int
clockintr_bar(struct clockframe *, char *, long);
You can also just do:
struct clockframe; int clockintr_foo(struct clockframe *, int,
short); int clockintr_bar(struct clockframe *, char *, long);
There is no need to have the full struct definition for a
pointer. With that there is no need to include machine/frame.h or
machine/cpu.h In my opinion this may be the preferred way of
handling this but unifying the definitions into one place still
makes sense to me.
That was the first thing I tried.
It doesn't work if clockframe is not a real struct. On some platforms
it's just a preprocessor macro. So you end up with a redefinition and
compilation fails, hence this patch.
That particular landmine is already in the tree, in systm.h. It seems
to have laid dormant due to #include ordering and luck.
I think there is also a discussion to be had about whether we could
just throw away the "clockframe" abstraction and put a
trapframe/intrframe/whatever pointer into cpu_info and rewrite the
CLKF macros to use a cpu_info pointer...
... but that seemed like more work than just making the
place-of-definition consistent.
Does that sound more appealing than "fixing" clockframe?
This is one of those annoying corners where there is too much unecessary
MD variation. Currently travelling without a laptop, so I can't easily
check the tree. But one note I wanted to make is that the definition of
struct clockframe and the CLKF_XXX macros should stay together.
