i386, arm, arm64, riscv: move ABI code from machdep.c to exec_machdep.c
ClosedPublic
Actions

Authored by kib on Oct 5 2021, 7:25 AM.

Details

Reviewers

jhb
jrtc27
andrew
manu
emaste
markj
imp

Commits

rGdfaf65023ef0: arm: move signal delivery code to exec_machdep.c
rGaba66031f27c: riscv: move signal delivery code to exec_machdep.c
rGb4ae6b166066: arm64: move signal delivery code to exec_machdep.c
rG4c5bf591522c: i386: move signal delivery code to exec_machdep.c

Summary

and ptrace bits to ptrace_machdep.c.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Oct 5 2021, 7:25 AM

Herald added a subscriber: imp. · View Herald TranscriptOct 5 2021, 7:25 AM

kib requested review of this revision.Oct 5 2021, 7:25 AM

kib retitled this revision from i386, arm, arm64, ruscv: move ABI code from machdep.c to exec_machdep.c to i386, arm, arm64, riscv: move ABI code from machdep.c to exec_machdep.c.Oct 5 2021, 11:05 AM

Looks good to me, though I didn't verify that the moved code was identical to the original.
Might want to do the ptrace move and the exec move as separate commits, but it's not terrible if not.

sys/arm/arm/exec_machdep.c
42–44 ↗	(On Diff #96260)	Any reason to retain this information?
sys/conf/files.arm64
51 ↗	(On Diff #96260)	This duplicates the entry below, so can be removed.

This revision is now accepted and ready to land.Oct 5 2021, 4:06 PM

In D32310#729364, @imp wrote:

Might want to do the ptrace move and the exec move as separate commits, but it's not terrible if not.

The diff is split per-arch, I do not plan to split it inside arches. Per commit view is available at https://kib.kiev.ua/git/gitweb.cgi?p=deviant3.git;a=shortlog;h=refs/heads/alloca

sys/arm/arm/exec_machdep.c
42–44 ↗	(On Diff #96260)	Removed. I just copied the whole blocks.

Same as @imp, I did not compare all of the removed/added blocks.

Couple of minor comments (and some positive observations) from reviewing the diff locally with --color-moved

sys/arm/arm/machdep.c
526 ↗	(On Diff #96260)	More K&R C gone :)
sys/i386/i386/exec_machdep.c
1400	This modified line is using tabs, not spaces like the rest of the function (which all your other modified returns in the function adhere to)
sys/i386/i386/machdep.c
737	These got lost, I assume deliberate because of where our SMP support is these days :)
sys/riscv/riscv/exec_machdep.c
192 ↗	(On Diff #96260)	You fixed the arm64 typo for this but not the riscv copy of it (as if we needed more reasons to point out how much of sys/riscv is a copy of sys/arm64 :P)
sys/riscv/riscv/machdep.c
548 ↗	(On Diff #96260)	This disappeared from the arm64 and riscv copies. I assume it's legacy crap that's no longer needed?

emaste added inline comments.Oct 5 2021, 6:49 PM

sys/arm/arm/machdep.c
526 ↗	(On Diff #96260)	\o/
sys/i386/i386/exec_machdep.c
541–547	I guess if we're doing other little cleanups like deleting `MPSAFE` we might as well rewrap this at a reasonable width?

kib marked 8 inline comments as done.Oct 6 2021, 3:23 AM

kib added inline comments.

sys/i386/i386/exec_machdep.c
1400	Emacs whitespace-mode only reported spaces to me. Anyway, I re-indented the function with tabs, and looked at other issues in this file highlighted by emacs.
sys/i386/i386/machdep.c
737	Yes, MPSAFE annotations are pointless today.
sys/riscv/riscv/machdep.c
548 ↗	(On Diff #96260)	Yes, I believe its perceived usefulness long time go. It is in fact detriment, since whatever changes syscalls.master and gen script take, SYSPROTO_H reminders are not updated.