Re: [Qemu-devel] [PATCH 1.1] qemu-ga: fix segv after failure to open log file

[Luiz Capitulino]

On Mon, 14 May 2012 17:04:17 -0500
Michael Roth <address@hidden> 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 <address@hidden>
> ---
>  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?

>          }
>      }
>