diff mbox series

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

Message ID 20191115131040.2834-3-robert.foley@linaro.org
State Superseded
Headers show
Series Make the qemu_logfile handle thread safe. | expand

Commit Message

Robert Foley Nov. 15, 2019, 1:10 p.m. UTC
Also added some explanation of the reasoning behind the branches.

Signed-off-by: Robert Foley <robert.foley@linaro.org>

---
v2
    - This is new in patch v2.
---
 util/log.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

-- 
2.17.1

Comments

Alex Bennée Nov. 18, 2019, 12:07 p.m. UTC | #1
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
diff mbox series

Patch

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)