From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49321) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUHQU-0002QL-6X for qemu-devel@nongnu.org; Tue, 15 May 2012 09:04:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SUHQN-0005lF-W9 for qemu-devel@nongnu.org; Tue, 15 May 2012 09:04:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53614) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SUHQN-0005kg-OV for qemu-devel@nongnu.org; Tue, 15 May 2012 09:04:23 -0400 Date: Tue, 15 May 2012 10:04:32 -0300 From: Luiz Capitulino Message-ID: <20120515100432.4999c2e7@doriath.home> In-Reply-To: <1337033057-31707-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1337033057-31707-1-git-send-email-mdroth@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1.1] qemu-ga: fix segv after failure to open log file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On Mon, 14 May 2012 17:04:17 -0500 Michael Roth wrote: > Currently, if we fail to open the specified log file (generally due to a > permissions issue), we'll assign NULL to the logfile handle (stderr, > initially) used by the logging routines, which can cause a segfault to > occur when we attempt to report the error before exiting. > > Instead, only re-assign if the open() was successful. > > Signed-off-by: Michael Roth > --- > qemu-ga.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/qemu-ga.c b/qemu-ga.c > index 3a88333..e2725c8 100644 > --- a/qemu-ga.c > +++ b/qemu-ga.c > @@ -681,6 +681,7 @@ int main(int argc, char **argv) > const char *log_filepath = NULL; > const char *pid_filepath = QGA_PIDFILE_DEFAULT; > const char *state_dir = QGA_STATEDIR_DEFAULT; > + FILE *log_file; > #ifdef _WIN32 > const char *service = NULL; > #endif > @@ -836,12 +837,13 @@ int main(int argc, char **argv) > become_daemon(pid_filepath); > } > if (log_filepath) { > - s->log_file = fopen(log_filepath, "a"); > - if (!s->log_file) { > + log_file = fopen(log_filepath, "a"); > + if (!log_file) { > g_critical("unable to open specified log file: %s", > strerror(errno)); > goto out_bad; > } > + s->log_file = log_file; Is it safe to change the log file this way? Isn't it necessary to go through g_log_set_default_handler() or some other function? > } > } >