From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4HQg-0005Km-1x for qemu-devel@nongnu.org; Thu, 15 Sep 2011 15:16:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R4HQd-00065c-GQ for qemu-devel@nongnu.org; Thu, 15 Sep 2011 15:16:58 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:63191) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R4HQd-00065R-BB for qemu-devel@nongnu.org; Thu, 15 Sep 2011 15:16:55 -0400 Received: by yib2 with SMTP id 2so2911882yib.4 for ; Thu, 15 Sep 2011 12:16:54 -0700 (PDT) Message-ID: <4E724F23.1020901@codemonkey.ws> Date: Thu, 15 Sep 2011 14:16:51 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1316107350-31172-1-git-send-email-lersek@redhat.com> In-Reply-To: <1316107350-31172-1-git-send-email-lersek@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC] main loop: fix some accesses made in sighandler context List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: qemu-devel@nongnu.org On 09/15/2011 12:22 PM, Laszlo Ersek wrote: > Make variables volatile ("sig_atomic_t" should cover "int" and "pid_t"). > > Also replace calls to functions that are not required to be async-signal-safe > [1]. (I haven't checked if any signal masks and/or previous suspension of the > interrupted thread keep the current calls safe.) > > termsig_handler() > -> qemu_system_killed(): shutdown_signal, shutdown_pid, no_shutdown [2] > -> qemu_system_shutdown_request(): shutdown_requested > -> qemu_notify_event() > -> qemu_event_increment(): fprintf(), strerror(), exit() > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03_03 > [2] http://lists.nongnu.org/archive/html/qemu-devel/2011-09/msg01757.html > > "checkpatch.pl" warned four times about "volatile", and considered the > zero-initialization of "no_shutdown" (which has static storage duration) an > error. > > Build tested only. Please CC me on any followup, I'm not subscribed. Thank you. > > Signed-off-by: Laszlo Ersek > --- > cpus.c | 13 ++++++++++--- > sysemu.h | 2 +- > vl.c | 6 +++--- > 3 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 54c188c..ed51247 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -289,9 +289,16 @@ static void qemu_event_increment(void) > > /* EAGAIN is fine, a read must be pending. */ > if (ret< 0&& errno != EAGAIN) { > - fprintf(stderr, "qemu_event_increment: write() failed: %s\n", > - strerror(errno)); > - exit (1); > + int len; > + char buf[128]; > + > + /* Don't bother with strerror_[rl]. Make a single attempt to write. */ > + len = snprintf(buf, sizeof buf, > + "qemu_event_increment: write() failed: %d\n", errno); I don't think you can rely on snprintf being signal safe. I think you should just exit on failure. OpenBSD lists snprintf as signal safe, but "probably not on other systems." Regards, Anthony Liguori