tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h

On 2019/11/18 16:52, Kamil Rytarowski wrote:
On 18.11.2019 05:38, Rin Okuyama wrote:
Thank you for your comment!

On 2019/11/17 22:42, Kamil Rytarowski wrote:
Please check it also with picotrace/truss:

netbsd32_signal.c needed to catch up with kern_sig.c so that syscall
information is provided with SIGTRAP TRAP_SCE/TRAP_SCX. I committed
the fix and picotrace/truss works fine now on COMPAT_NETBSD32.

Thanks! I have submitted a mail how to further improve it.

Thank you for your advice! I committed it!

On 2019/11/17 22:42, Kamil Rytarowski wrote:
On 17.11.2019 04:34, Rin Okuyama wrote:

In order to distangle circular dependency between
<sys/compat/sys/siginfo.h> v.s. <machine/netbsd32_machdep.h>,
I propose

(1) Move NETBSD32_INT64_ALIGN from <machine/netbsd32_machdep.h> to

(2) Move netbsd32_{,u}int64 from <sys/compat/netbsd32/netbsd32.h> to

See attached patch for example on amd64.

What do you think about duplicating the defines of netbsd32_uint64
inside sys/compat/sys/siginfo.h + adding a comment about keeping it in
sync with netbsd32.h?

I think that avoiding spaghetti dependencies is a benefit.

We already duplicated there _ptrace_state, removing circular
dependencies between sys/ptrace.h and sys/siginfo.h.

I don't think this is a good idea in this case. If we want to have
duplicate define of netbsd32_uint64, and to avoid an "#ifdef __x86_64__"
mess in <compat/sys/siginfo.h>, we need to move NETBSD32_INT64_ALIGN to
<machine/somewhere.h> other than <machine/netbsd32_machdep.h>. If so,
why not <machine/types.h>?

No need to move anything out of machine/netbsd32_machdep.h. It's
sufficient to define netbsd32_int64 with a custom non-conflicting name
or protect it with #ifdef before typedefing/defining twice.

Also, in my proposal, spaghetti dependencies are avoided in the end;
everyone depends on <sys/types.h>, and not on each other.

However I have no strong opinions here. I would personally avoid
compat32 definitions in sys/types.h.

Compat code tends to need hacks, so it is sensible imho to restrict them
to compat headers (I am aware that it's not always followed).

I agree with you on that compat codes should not be put into sys headers
as far as possible. However, I still do not understand what you mean.

(1) NETBSD32_INT64_ALIGN == __attribute__((__aligned__(4))) is in
    <machine/netbsd32_machdep.h>, and

(2) <machine/netbsd32_machdep.h> needs <compat/sys/siginfo.h>

Therefore, we cannot use NETBSD32_INT64_ALIGN in <compat/sys/siginfo.h>.
Of course, we can typedef _netbsd32_uint64 in <compat/sys/siginfo.h> like:

typedef uint64_t _netbsd32_uint64
#ifdef __x86_64__

Do you think it is OK? I guess that I miss some elementary things...
Sorry in advance ;-).


Home | Main Index | Thread Index | Old Index