On 11/8/21 3:37 AM, Warner Losh wrote:
> All the *-users generally use the Linux style of negative return codes
> for errno. However, other systems, like FreeBSD, have a different
> convention. Allow those systems to insert code after the syscall that
> adjusts the return value of the system call to match the native linux
> format.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
> common-user/host/aarch64/safe-syscall.inc.S | 1 +
> common-user/host/arm/safe-syscall.inc.S | 1 +
> common-user/host/i386/safe-syscall.inc.S | 1 +
> common-user/host/ppc64/safe-syscall.inc.S | 1 +
> common-user/host/riscv/safe-syscall.inc.S | 1 +
> common-user/host/s390x/safe-syscall.inc.S | 1 +
> common-user/host/x86_64/safe-syscall.inc.S | 1 +
> linux-user/safe-syscall.S | 1 +
> 8 files changed, 8 insertions(+)
>
> diff --git a/common-user/host/aarch64/safe-syscall.inc.S b/common-user/host/aarch64/safe-syscall.inc.S
> index bc1f5a9792..81d83e8e79 100644
> --- a/common-user/host/aarch64/safe-syscall.inc.S
> +++ b/common-user/host/aarch64/safe-syscall.inc.S
> @@ -64,6 +64,7 @@ safe_syscall_start:
> svc 0x0
> safe_syscall_end:
> /* code path for having successfully executed the syscall */
> + ADJUST_SYSCALL_RETCODE
> ret
>
> 0:
Not sure about this, really. Is it really that much cleaner to insert this than create
separate 10-line files, with the adjustment included?
While the meat of these files is only around 10 lines, the files themselves are more like 70 lines with a lot of extra marking to ensure proper integration with toolchains. Such lines have a tendency, over time, to need adjusting as new toolchains change the requirements slightly (clang certainly has forced that in a number of places in FreeBSD's code base, and every new version seems to require something). The adjustments have all been 3 lines (gmail seems to hate my formatting):
+#define ADJUST_SYSCALL_RETCODE \
+ jnb 2f; \
+ neg %rax; \
+ 2:
which is significantly easier to maintain than having to monitor these files for changes and copying over the changes that happen. Plus, as I'm upstreaming the arch support, it's one more file I have to include in the review, it's one more file that may get questions and advice I'll have to answer with 'it's a verbatim copy of the linux version with these three lines added, why do I need to make those stylistic changes'. All of which takes extra time. The upstreaming is going much more slowly than I'd anticipated (but on the up-side also finding more problems than I thought were latent in the system), and we're still at about 30k lines (after starting at about 36k lines, though some of that is due to deleting mips). It's been running about a month per 1000 lines to get reviewed and upstreamed. There's about 626 lines in these 6 files, so sharing them seemed like it would save me a few weeks of upstreaming time as well (though I'll be the first to admit that translating LoC metrics to time has a dubious history).
The other alternative I considered was having a #ifdef __FreeBSD__ .. #endif in all those files, but I thought that even more intrusive.
Warner