Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On Mon, Feb 17, 2014 at 06:00:03PM +0100, Markus Armbruster wrote:
Paolo Bonzini pbonz...@redhat.com writes:
Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto:
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
+}
+
void qtest_quit(QTestState *s)
{
int status;
if (s-qemu_pid != -1) {
kill(s-qemu_pid, SIGTERM);
waitpid(s-qemu_pid, status, 0);
}
close(s-fd);
close(s-qmp_fd);
g_string_free(s-rx, true);
g_free(s);
}
Not async-signal safe. You need to ignore the g_string_free and
g_free (perhaps even the closes) if calling from the sigabrt_handler.
kill(), waitpid() and close() are all async-signal-safe.
SIGABRT is normally synchronous enough: it's sent by abort(). But of
course, nothing stops the user from kill -ABRT. Or GLib from calling
abort() in some place where an attempt to reenter it crashes burns.
Not sure I'd care, but I'm pretty sure I don't care for freeing stuff on
exit :)
Yes, SIGABRT is synchronous for all purposes. So the only danger is
that g_string_free() or g_free() could fail while we're in
g_assert(false). But they don't, which makes sense because they are
totally unrelated to g_assert() and therefore can handle re-entrancy.
In practice there is no issue and I've tested assertion failure with
glib 1.2.10.
Stefan
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote:
Stefan Hajnoczi stefa...@redhat.com 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
Stefan Hajnoczi stefa...@gmail.com writes:
On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote:
Stefan Hajnoczi stefa...@redhat.com 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.
For me, it's in the same category as free(NULL).
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Markus Armbruster arm...@redhat.com writes:
Stefan Hajnoczi stefa...@redhat.com writes:
The QEMU process stays running if the test case fails. This patch fixes
the leak by installing a SIGABRT signal handler which invokes
qtest_end().
In order to make that work for assertion failures during qtest_init(),
we need to initialize QTestState fields including file descriptors and
pids carefully. qtest_quit() is then safe to call even during
qtest_init().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/libqtest.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 8b2b2d7..09a0481 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -44,6 +44,7 @@ struct QTestState
bool irq_level[MAX_IRQ];
GString *rx;
pid_t qemu_pid; /* our child QEMU process */
+struct sigaction sigact_old; /* restored on exit */
};
#define g_assert_no_errno(ret) do { \
@@ -88,6 +89,11 @@ static int socket_accept(int sock)
return ret;
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
Don't you have to re-raise SIGABRT here, to actually terminate the
process?
+}
+
QTestState *qtest_init(const char *extra_args)
{
QTestState *s;
[...]
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Stefan Hajnoczi stefa...@gmail.com writes:
On Mon, Feb 17, 2014 at 06:00:03PM +0100, Markus Armbruster wrote:
Paolo Bonzini pbonz...@redhat.com writes:
Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto:
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
+}
+
void qtest_quit(QTestState *s)
{
int status;
if (s-qemu_pid != -1) {
kill(s-qemu_pid, SIGTERM);
waitpid(s-qemu_pid, status, 0);
}
close(s-fd);
close(s-qmp_fd);
g_string_free(s-rx, true);
g_free(s);
}
Not async-signal safe. You need to ignore the g_string_free and
g_free (perhaps even the closes) if calling from the sigabrt_handler.
kill(), waitpid() and close() are all async-signal-safe.
SIGABRT is normally synchronous enough: it's sent by abort(). But of
course, nothing stops the user from kill -ABRT. Or GLib from calling
abort() in some place where an attempt to reenter it crashes burns.
Not sure I'd care, but I'm pretty sure I don't care for freeing stuff on
exit :)
Yes, SIGABRT is synchronous for all purposes. So the only danger is
that g_string_free() or g_free() could fail while we're in
g_assert(false). But they don't, which makes sense because they are
totally unrelated to g_assert() and therefore can handle re-entrancy.
The (theoretical!) problem is when it aborts in the bowels of g_*free(),
and your SIGABRT handler reenters g_*free().
In practice there is no issue and I've tested assertion failure with
glib 1.2.10.
Worst that can happen is we crash on the way from abort() to process
termination. Tolerable.
Still, avoiding unnecessary cleanup on that path seems prudent to me.
If you agree, factor out the kill()/waitpid(), and call only that from
the signal handler.
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Il 18/02/2014 10:05, Stefan Hajnoczi ha scritto: SIGABRT is normally synchronous enough: it's sent by abort(). But of course, nothing stops the user from kill -ABRT. Or GLib from calling abort() in some place where an attempt to reenter it crashes burns. Not sure I'd care, but I'm pretty sure I don't care for freeing stuff on exit :) Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and therefore can handle re-entrancy. If malloc aborts due to a double free or other similar problem, you may risk reentering it. Paolo
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On Tue, Feb 18, 2014 at 11:07:53AM +0100, Paolo Bonzini wrote: Il 18/02/2014 10:05, Stefan Hajnoczi ha scritto: SIGABRT is normally synchronous enough: it's sent by abort(). But of course, nothing stops the user from kill -ABRT. Or GLib from calling abort() in some place where an attempt to reenter it crashes burns. Not sure I'd care, but I'm pretty sure I don't care for freeing stuff on exit :) Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and therefore can handle re-entrancy. If malloc aborts due to a double free or other similar problem, you may risk reentering it. If you register the custom SIGABRT handler with sigaction + SA_RESETHAND then you'd avoid the re-entrancy risk, since a cascading SIGABRT would get handled by the system default handler, which would immediately terminate the process. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Il 18/02/2014 11:17, Daniel P. Berrange ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and therefore can handle re-entrancy. If malloc aborts due to a double free or other similar problem, you may risk reentering it. If you register the custom SIGABRT handler with sigaction + SA_RESETHAND then you'd avoid the re-entrancy risk, since a cascading SIGABRT would get handled by the system default handler, which would immediately terminate the process. I meant reentering malloc. Paolo
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Il 18/02/2014 11:05, Markus Armbruster ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and therefore can handle re-entrancy. The (theoretical!) problem is when it aborts in the bowels of g_*free(), and your SIGABRT handler reenters g_*free(). In practice there is no issue and I've tested assertion failure with glib 1.2.10. Worst that can happen is we crash on the way from abort() to process termination. Tolerable. What about recursive locking of a non-recursive mutex? Paolo
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Paolo Bonzini pbonz...@redhat.com writes: Il 18/02/2014 11:05, Markus Armbruster ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and therefore can handle re-entrancy. The (theoretical!) problem is when it aborts in the bowels of g_*free(), and your SIGABRT handler reenters g_*free(). In practice there is no issue and I've tested assertion failure with glib 1.2.10. Worst that can happen is we crash on the way from abort() to process termination. Tolerable. What about recursive locking of a non-recursive mutex? You're right, deadlock is possible. Strengthens the argument for: Still, avoiding unnecessary cleanup on that path seems prudent to me. If you agree, factor out the kill()/waitpid(), and call only that from the signal handler.
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On Tue, Feb 18, 2014 at 11:02:52AM +0100, Markus Armbruster wrote:
Markus Armbruster arm...@redhat.com writes:
Stefan Hajnoczi stefa...@redhat.com writes:
The QEMU process stays running if the test case fails. This patch fixes
the leak by installing a SIGABRT signal handler which invokes
qtest_end().
In order to make that work for assertion failures during qtest_init(),
we need to initialize QTestState fields including file descriptors and
pids carefully. qtest_quit() is then safe to call even during
qtest_init().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/libqtest.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 8b2b2d7..09a0481 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -44,6 +44,7 @@ struct QTestState
bool irq_level[MAX_IRQ];
GString *rx;
pid_t qemu_pid; /* our child QEMU process */
+struct sigaction sigact_old; /* restored on exit */
};
#define g_assert_no_errno(ret) do { \
@@ -88,6 +89,11 @@ static int socket_accept(int sock)
return ret;
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
Don't you have to re-raise SIGABRT here, to actually terminate the
process?
No. POSIX says:
RETURN VALUE
The abort() function shall not return.
(BTW the way to avoid that is using longjmp.)
The Linux man page is more explicit:
If the SIGABRT signal is ignored, or caught by a handler that returns,
the abort() function will still terminate the process. It does this by
restoring the default disposition for SIGABRT and then raising the signal
for a second time.
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On Tue, Feb 18, 2014 at 10:55:56AM +0100, Markus Armbruster wrote:
Stefan Hajnoczi stefa...@gmail.com writes:
On Mon, Feb 17, 2014 at 05:49:31PM +0100, Markus Armbruster wrote:
Stefan Hajnoczi stefa...@redhat.com writes:
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.
For me, it's in the same category as free(NULL).
Understood. I'll drop it from the next patch.
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On Tue, Feb 18, 2014 at 11:43:10AM +0100, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Il 18/02/2014 11:05, Markus Armbruster ha scritto: Yes, SIGABRT is synchronous for all purposes. So the only danger is that g_string_free() or g_free() could fail while we're in g_assert(false). But they don't, which makes sense because they are totally unrelated to g_assert() and therefore can handle re-entrancy. The (theoretical!) problem is when it aborts in the bowels of g_*free(), and your SIGABRT handler reenters g_*free(). In practice there is no issue and I've tested assertion failure with glib 1.2.10. Worst that can happen is we crash on the way from abort() to process termination. Tolerable. What about recursive locking of a non-recursive mutex? You're right, deadlock is possible. Strengthens the argument for: Still, avoiding unnecessary cleanup on that path seems prudent to me. If you agree, factor out the kill()/waitpid(), and call only that from the signal handler. Okay, I'm convinced.
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Stefan Hajnoczi stefa...@gmail.com writes:
On Tue, Feb 18, 2014 at 11:02:52AM +0100, Markus Armbruster wrote:
Markus Armbruster arm...@redhat.com writes:
Stefan Hajnoczi stefa...@redhat.com writes:
The QEMU process stays running if the test case fails. This patch fixes
the leak by installing a SIGABRT signal handler which invokes
qtest_end().
In order to make that work for assertion failures during qtest_init(),
we need to initialize QTestState fields including file descriptors and
pids carefully. qtest_quit() is then safe to call even during
qtest_init().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/libqtest.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 8b2b2d7..09a0481 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -44,6 +44,7 @@ struct QTestState
bool irq_level[MAX_IRQ];
GString *rx;
pid_t qemu_pid; /* our child QEMU process */
+struct sigaction sigact_old; /* restored on exit */
};
#define g_assert_no_errno(ret) do { \
@@ -88,6 +89,11 @@ static int socket_accept(int sock)
return ret;
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
Don't you have to re-raise SIGABRT here, to actually terminate the
process?
No. POSIX says:
RETURN VALUE
The abort() function shall not return.
(BTW the way to avoid that is using longjmp.)
The Linux man page is more explicit:
If the SIGABRT signal is ignored, or caught by a handler that returns,
the abort() function will still terminate the process. It does this by
restoring the default disposition for SIGABRT and then raising the signal
for a second time.
Learn something new :) Thanks!
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
On 18 February 2014 09:17, Stefan Hajnoczi stefa...@gmail.com wrote: 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. As an aside, ever try stracing gtester? It implements its -o file option by always doing the write of the XML, so if you didn't specify -o then the write()s and close() are all done on fd -1... thanks -- PMM
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto:
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
+}
+
void qtest_quit(QTestState *s)
{
int status;
if (s-qemu_pid != -1) {
kill(s-qemu_pid, SIGTERM);
waitpid(s-qemu_pid, status, 0);
}
close(s-fd);
close(s-qmp_fd);
g_string_free(s-rx, true);
g_free(s);
}
Not async-signal safe. You need to ignore the g_string_free and g_free
(perhaps even the closes) if calling from the sigabrt_handler.
Paolo
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Stefan Hajnoczi stefa...@redhat.com writes:
The QEMU process stays running if the test case fails. This patch fixes
the leak by installing a SIGABRT signal handler which invokes
qtest_end().
In order to make that work for assertion failures during qtest_init(),
we need to initialize QTestState fields including file descriptors and
pids carefully. qtest_quit() is then safe to call even during
qtest_init().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/libqtest.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 8b2b2d7..09a0481 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -44,6 +44,7 @@ struct QTestState
bool irq_level[MAX_IRQ];
GString *rx;
pid_t qemu_pid; /* our child QEMU process */
+struct sigaction sigact_old; /* restored on exit */
};
#define g_assert_no_errno(ret) do { \
@@ -88,6 +89,11 @@ static int socket_accept(int sock)
return ret;
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
+}
+
QTestState *qtest_init(const char *extra_args)
{
QTestState *s;
@@ -96,11 +102,22 @@ QTestState *qtest_init(const char *extra_args)
gchar *qmp_socket_path;
gchar *command;
const char *qemu_binary;
+struct sigaction sigact;
qemu_binary = getenv(QTEST_QEMU_BINARY);
g_assert(qemu_binary != NULL);
-s = g_malloc(sizeof(*s));
+s = g_malloc0(sizeof(*s));
+s-fd = -1;
+s-qmp_fd = -1;
+s-qemu_pid = -1;
+
+/* 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.
+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?
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).
g_string_free(s-rx, true);
g_free(s);
}
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Il 17/02/2014 17:49, Markus Armbruster ha scritto: 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. Certainly memset of struct sigaction or sigset_t * is common enough that no one in their right minds would do this. Is there really an OS that does it? Also, the above justification is quite feeble; it would work for binary compatibility of sigset_t* arguments, but not for embedded sigset_t structs. I'm CCing our resident POSIX experts in hope that this paragraph can be eliminated from the standard. :) Related to this, there are a bunch of Coverity reports where we use uninitialized fields of a struct sigaction. Paolo
Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure
Paolo Bonzini pbonz...@redhat.com writes:
Il 17/02/2014 16:44, Stefan Hajnoczi ha scritto:
}
+static void sigabrt_handler(int signo)
+{
+qtest_end();
+}
+
void qtest_quit(QTestState *s)
{
int status;
if (s-qemu_pid != -1) {
kill(s-qemu_pid, SIGTERM);
waitpid(s-qemu_pid, status, 0);
}
close(s-fd);
close(s-qmp_fd);
g_string_free(s-rx, true);
g_free(s);
}
Not async-signal safe. You need to ignore the g_string_free and
g_free (perhaps even the closes) if calling from the sigabrt_handler.
kill(), waitpid() and close() are all async-signal-safe.
SIGABRT is normally synchronous enough: it's sent by abort(). But of
course, nothing stops the user from kill -ABRT. Or GLib from calling
abort() in some place where an attempt to reenter it crashes burns.
Not sure I'd care, but I'm pretty sure I don't care for freeing stuff on
exit :)
