[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() fail
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure |
Date: |
Tue, 18 Feb 2014 10:17:46 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote:
> Stefan Hajnoczi <address@hidden> writes:
> > + /* Catch SIGABRT to clean up on g_assert() failure */
> > + sigact = (struct sigaction){
> > + .sa_handler = sigabrt_handler,
> > + .sa_flags = SA_RESETHAND,
> > + };
>
> Assumes zero-initialization has the same effect as
> sigemptyset(&sigact.sa_mask). Quoting POSIX:
>
> The implementation of the sigemptyset() (or sigfillset()) function
> could quite trivially clear (or set) all the bits in the signal set.
> Alternatively, it would be reasonable to initialize part of the
> structure, such as a version field, to permit binary-compatibility
> between releases where the size of the set varies. For such
> reasons, either sigemptyset() or sigfillset() must be called prior
> to any other use of the signal set, even if such use is read-only
> (for example, as an argument to sigpending()).
>
> Looks like you better sigemptyset() here, for maximum portability.
Okay, will do that.
> > + sigaction(SIGABRT, &sigact, &s->sigact_old);
> >
> > socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
> > qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
> > @@ -150,13 +167,19 @@ void qtest_quit(QTestState *s)
> > {
> > int status;
> >
> > + sigaction(SIGABRT, &s->sigact_old, NULL);
> > +
>
> Can this ever restore the action to anything but the default action?
This code is in a "library" so I don't want to make assumptions about
the callers. Keeping sigact_old around is literally 1 line of code so I
think it's worth it.
> > if (s->qemu_pid != -1) {
> > kill(s->qemu_pid, SIGTERM);
> > waitpid(s->qemu_pid, &status, 0);
> > }
> >
> > - close(s->fd);
> > - close(s->qmp_fd);
> > + if (s->fd != -1) {
> > + close(s->fd);
> > + }
> > + if (s->qmp_fd != -1) {
> > + close(s->qmp_fd);
> > + }
>
> I generally don't bother to avoid close(-1).
When I drive on the highway I stay on the lanes but I guess I could just
slide along the side barriers :). It's a style issue but close(-1)
annoys me in strace so I try to avoid doing it.
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, (continued)
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Stefan Hajnoczi, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Markus Armbruster, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Paolo Bonzini, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Markus Armbruster, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Stefan Hajnoczi, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Paolo Bonzini, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Daniel P. Berrange, 2014/02/18
- Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Paolo Bonzini, 2014/02/18
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Markus Armbruster, 2014/02/17
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Peter Maydell, 2014/02/18
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Markus Armbruster, 2014/02/18
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Stefan Hajnoczi, 2014/02/18
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure, Markus Armbruster, 2014/02/18