Re: [PATCH v2 2/6] Cleaned up flow of code in qemu_set_log(), to simplify and clarify.

["Alex Bennée" <alex.bennee@linaro.org>]

Robert Foley <robert.foley@linaro.org> writes:

> Also added some explanation of the reasoning behind the branches.
>
> Signed-off-by: Robert Foley <robert.foley@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
> v2
>     - This is new in patch v2.
> ---
>  util/log.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/util/log.c b/util/log.c
> index 4316fe74ee..417d16ec66 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -54,12 +54,25 @@ static bool log_uses_own_buffers;
>  /* enable or disable low levels log */
>  void qemu_set_log(int log_flags)
>  {
> +    bool need_to_open_file = false;
>      qemu_loglevel = log_flags;
>  #ifdef CONFIG_TRACE_LOG
>      qemu_loglevel |= LOG_TRACE;
>  #endif
> -    if (!qemu_logfile &&
> -        (is_daemonized() ? logfilename != NULL : qemu_loglevel)) {
> +    /*
> +     * In all cases we only log if qemu_loglevel is set.
> +     * Also:
> +     *   If not daemonized we will always log either to stderr
> +     *     or to a file (if there is a logfilename).
> +     *   If we are daemonized,
> +     *     we will only log if there is a logfilename.
> +     */
> +    if (qemu_loglevel && (!is_daemonized() || logfilename)) {
> +        need_to_open_file = true;
> +    }
> +    if (qemu_logfile && !need_to_open_file) {
> +        qemu_log_close();
> +    } else if (!qemu_logfile && need_to_open_file) {
>          if (logfilename) {
>              qemu_logfile = fopen(logfilename, log_append ? "a" : "w");
>              if (!qemu_logfile) {
> @@ -93,10 +106,6 @@ void qemu_set_log(int log_flags)
>              log_append = 1;
>          }
>      }
> -    if (qemu_logfile &&
> -        (is_daemonized() ? logfilename == NULL : !qemu_loglevel)) {
> -        qemu_log_close();
> -    }
>  }
>
>  void qemu_log_needs_buffers(void)


--
Alex Bennée