Re: [PATCH 14/14] bsd-user/signal: Create a dummy signal queueing function

From: Warner Losh <imp@bsdimp.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: "Kyle Evans" <kevans@freebsd.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [PATCH 14/14] bsd-user/signal: Create a dummy signal queueing function
Date: Fri, 24 Sep 2021 14:11:26 -0600	[thread overview]
Message-ID: <CANCZdfpNFhFQk0inNhtb6Ft4vdKtar3jBBjpL6_KfTpf9yj4dA@mail.gmail.com> (raw)
In-Reply-To: <1f21d9a3-fe08-22e7-45f9-27dae0c1c6ba@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 2715 bytes --]

On Fri, Sep 24, 2021 at 6:00 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 9/21/21 11:14 PM, Warner Losh wrote:
> > Create dummy signal queueing function so we can start to integrate other
> > architectures (at the cost of signals remaining broken) to tame the
> > dependency graph a bit and to bring in signals in a more controlled
> > fashion.
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/qemu.h   | 1 +
> >   bsd-user/signal.c | 8 ++++++++
> >   2 files changed, 9 insertions(+)
> >
> > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> > index 5a2fd87e44..85d1f8fd2a 100644
> > --- a/bsd-user/qemu.h
> > +++ b/bsd-user/qemu.h
> > @@ -209,6 +209,7 @@ void process_pending_signals(CPUArchState *cpu_env);
> >   void signal_init(void);
> >   long do_sigreturn(CPUArchState *env);
> >   long do_rt_sigreturn(CPUArchState *env);
> > +int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
> >   abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr,
> abi_ulong sp);
> >
> >   /* mmap.c */
> > diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> > index ad6d935569..4e7f618944 100644
> > --- a/bsd-user/signal.c
> > +++ b/bsd-user/signal.c
> > @@ -19,6 +19,14 @@
> >   #include "qemu/osdep.h"
> >
> >   #include "qemu.h"
> > +/*
>
> Whacky whitespace.
>

fixed.

> > + * Queue a signal so that it will be send to the virtual CPU as soon as
> > + * possible.
> > + */
> > +int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info)
> > +{
> > +    return 1;
> > +}
>
> Both here and in linux-user, there are no error conditions.  We should
> change the return
> to void.
>

I'll prep a patch to follow up for both linux and bsd user.

> Also, consider folding in the signal-common.h cleanup soon.
> But don't let either hold you up too much with rebasing.
>

It's on my list. This 'dummy' routine is just to get things linking to
help simplify the rather tangled dependency tree to get things
in, still have them compile and still have at least simple hello
world continue to work. Behind these reviews are three streams
of patches for 3 more architectures: arm, aarch64 and riscv64.

I'll create a patch for both linux-user and fix in bsd-user as part of the
signal.c upstreaming I'm working on.

It brings to mind something else... There's times it might be easier
to refactor between bsd-user and linux-user rather than upstream
something that's largely copied from linux-user. Is there a good
way to do that and talk about the design before I sink a ton of time
into something that's the wrong direction?

Thanks for the reviews and feedback.

Warner

> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> r~
>

[-- Attachment #2: Type: text/html, Size: 4236 bytes --]