qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: Warner Losh <imp@bsdimp.com>
Cc: "Philippe Mathieu-Daude" <f4bug@amsat.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Laurent Vivier" <laurent@vivier.eu>
Subject: Re: [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call
Date: Tue, 9 Nov 2021 09:11:34 +0100	[thread overview]
Message-ID: <22fa61a3-c8ba-e0fe-36b8-86ba1c90ca84@linaro.org> (raw)
In-Reply-To: <CANCZdfohHLKjstby1t3vA3u=MU2qdt_FXNTSpWyUPbbyd2p5aw@mail.gmail.com>

On 11/8/21 7:49 PM, Warner Losh wrote:
>      >       /* 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?
...
> 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.
...
> The other alternative I considered was having a #ifdef __FreeBSD__ .. #endif in all those 
> files, but I thought that even more intrusive.

Actually, the ifdef sounds surprisingly attractive to me.  Is it ENOCOFFEE?

What I find awkward about ADJUST_SYSCALL_RETCODE is that when you're looking at the 
definition, you have no reference to the context, and vice versa.  Not that it can't be 
worked out, but it seems like the same amount of code either way, and clearer when it's 
together.

We've already split the host cpu apart, which is the major point of ifdeffery, so it 
doesn't seem like we'll wind up with a large amount of ifdefs here; we're not likely to 
see mynewos-user wanting to share this code any time soon.

I feel sufficiently fuzzy on this to solicit other opinions though.


r~


  reply	other threads:[~2021-11-09  8:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-08  2:37 [RFC 0/4] linux-user: simplify safe signal handling Warner Losh
2021-11-08  2:37 ` [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext Warner Losh
2021-11-08 15:03   ` Richard Henderson
2021-11-10 15:43   ` Philippe Mathieu-Daudé
2021-11-08  2:37 ` [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall Warner Losh
2021-11-08 15:07   ` Richard Henderson
2021-11-08 16:39     ` Warner Losh
2021-11-10 16:20       ` Warner Losh
2021-11-10 16:31         ` Richard Henderson
2021-11-10 15:44   ` Philippe Mathieu-Daudé
2021-11-08  2:37 ` [RFC 3/4] linux-user/safe-syscall.inc.S: Move to common-user Warner Losh
2021-11-08  2:37 ` [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call Warner Losh
2021-11-08 15:10   ` Richard Henderson
2021-11-08 18:49     ` Warner Losh
2021-11-09  8:11       ` Richard Henderson [this message]
2021-11-10  2:10         ` Warner Losh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22fa61a3-c8ba-e0fe-36b8-86ba1c90ca84@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=f4bug@amsat.org \
    --cc=imp@bsdimp.com \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).