diff mbox series

qemu_set_log_filename: filename argument may be NULL

Message ID 20200123193626.19956-1-salvador@qindel.com
State New
Headers show
Series qemu_set_log_filename: filename argument may be NULL | expand

Commit Message

Salvador Fandiño Jan. 23, 2020, 7:36 p.m. UTC 
From: Salvador Fandino <salvador@qindel.com>

NULL is a valid log filename used to indicate we want to use stderr
but qemu_set_log_filename (which is called by bsd-user/main.c) was not
handling it correctly.

That also made redundant a couple of NULL checks in calling code which
have been removed.

Signed-off-by: Salvador Fandino <salvador@qindel.com>
---
 trace/control.c |  4 +---
 util/log.c      | 28 ++++++++++++++++------------
 vl.c            |  5 +----
 3 files changed, 18 insertions(+), 19 deletions(-)

Comments

Stefan Hajnoczi Jan. 24, 2020, 10:23 a.m. UTC | #1 
On Thu, Jan 23, 2020 at 08:36:26PM +0100, salvador@qindel.com wrote:
> From: Salvador Fandino <salvador@qindel.com>
> 
> NULL is a valid log filename used to indicate we want to use stderr
> but qemu_set_log_filename (which is called by bsd-user/main.c) was not
> handling it correctly.
> 
> That also made redundant a couple of NULL checks in calling code which
> have been removed.
> 
> Signed-off-by: Salvador Fandino <salvador@qindel.com>
> ---
>  trace/control.c |  4 +---
>  util/log.c      | 28 ++++++++++++++++------------
>  vl.c            |  5 +----
>  3 files changed, 18 insertions(+), 19 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi Jan. 24, 2020, 10:24 a.m. UTC | #2 
On Thu, Jan 23, 2020 at 08:36:26PM +0100, salvador@qindel.com wrote:
> From: Salvador Fandino <salvador@qindel.com>
> 
> NULL is a valid log filename used to indicate we want to use stderr
> but qemu_set_log_filename (which is called by bsd-user/main.c) was not
> handling it correctly.
> 
> That also made redundant a couple of NULL checks in calling code which
> have been removed.
> 
> Signed-off-by: Salvador Fandino <salvador@qindel.com>
> ---
>  trace/control.c |  4 +---
>  util/log.c      | 28 ++++++++++++++++------------
>  vl.c            |  5 +----
>  3 files changed, 18 insertions(+), 19 deletions(-)

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan
diff mbox series

Patch

diff --git a/trace/control.c b/trace/control.c
index 0fb8124160..6c775e68eb 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -229,9 +229,7 @@  void trace_init_file(const char *file)
     /* If both the simple and the log backends are enabled, "--trace file"
      * only applies to the simple backend; use "-D" for the log backend.
      */
-    if (file) {
-        qemu_set_log_filename(file, &error_fatal);
-    }
+    qemu_set_log_filename(file, &error_fatal);
 #else
     if (file) {
         fprintf(stderr, "error: --trace file=...: "
diff --git a/util/log.c b/util/log.c
index 867264da8d..47f2827397 100644
--- a/util/log.c
+++ b/util/log.c
@@ -148,25 +148,29 @@  void qemu_log_needs_buffers(void)
  * Allow the user to include %d in their logfile which will be
  * substituted with the current PID. This is useful for debugging many
  * nested linux-user tasks but will result in lots of logs.
+ *
+ * filename may be NULL. In that case, log output is sent to stderr
  */
 void qemu_set_log_filename(const char *filename, Error **errp)
 {
-    char *pidstr;
     g_free(logfilename);
     logfilename = NULL;
 
-    pidstr = strstr(filename, "%");
-    if (pidstr) {
-        /* We only accept one %d, no other format strings */
-        if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
-            error_setg(errp, "Bad logfile format: %s", filename);
-            return;
-        } else {
-            logfilename = g_strdup_printf(filename, getpid());
-        }
-    } else {
-        logfilename = g_strdup(filename);
+    if (filename) {
+            char *pidstr = strstr(filename, "%");
+            if (pidstr) {
+                /* We only accept one %d, no other format strings */
+                if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
+                    error_setg(errp, "Bad logfile format: %s", filename);
+                    return;
+                } else {
+                    logfilename = g_strdup_printf(filename, getpid());
+                }
+            } else {
+                logfilename = g_strdup(filename);
+            }
     }
+
     qemu_log_close();
     qemu_set_log(qemu_loglevel);
 }
diff --git a/vl.c b/vl.c
index 71d3e7eefb..affe896c6e 100644
--- a/vl.c
+++ b/vl.c
@@ -3886,10 +3886,7 @@  int main(int argc, char **argv, char **envp)
 
     /* Open the logfile at this point and set the log mask if necessary.
      */
-    if (log_file) {
-        qemu_set_log_filename(log_file, &error_fatal);
-    }
-
+    qemu_set_log_filename(log_file, &error_fatal);
     if (log_mask) {
         int mask;
         mask = qemu_str_to_log_mask(log_mask);