qemu-riscv
[Top][All Lists]
Advanced

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

Re: [PATCH v2 3/3] gdbstub: replace exit(0) with proper shutdown


From: Peter Maydell
Subject: Re: [PATCH v2 3/3] gdbstub: replace exit(0) with proper shutdown
Date: Mon, 4 Sep 2023 10:41:55 +0100

On Mon, 4 Sept 2023 at 10:36, Clément Chigot <chigot@adacore.com> wrote:
>
> On Mon, Sep 4, 2023 at 11:23 AM Peter Maydell <peter.maydell@linaro.org> 
> wrote:
> >
> > On Wed, 23 Aug 2023 at 08:07, Clément Chigot <chigot@adacore.com> wrote:
> > >
> > > This replaces the exit(0) call by a shutdown request, ensuring a proper
> > > cleanup of Qemu. Otherwise, some connections could be broken without
> > > being correctly flushed.
> > >
> > > Signed-off-by: Clément Chigot <chigot@adacore.com>

> > > +    /*
> > > +     * Shutdown request is a clean way to stop the QEMU, compared
> > > +     * to a direct call to exit(). But we can't pass the exit code
> > > +     * through it so avoid doing that when it can matter.
> > > +     * As this function is also called during the cleanup process,
> > > +     * avoid sending the request if one is already set.
> > > +     */
> > > +    if (code) {
> > > +        exit(code);
> > > +    } else if (!qemu_shutdown_requested_get()) {
> > > +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> > > +    }
> > >  }
> >
> > This definitely doesn't look right. Either exit() is OK
> > to call, or it is not. We shouldn't be exiting one way
> > if the exit status is 0 and another way if it is non-0.
>
> I do agree but AFAIK, this isn't possible to pass the exit code using
> qemu_system_shutdown_request.

That would mean that we should add a mechanism to do so.

But my opinion is still what I said about the first version
of this patchset: we should fix whatever the problem is
that means that gdb_exit() is not correctly ensuring that
gdb gets the packet response, not paper over it like this.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]