diff mbox series

qemu_set_log_filename: filename argument may be NULL

Message ID 20200122210812.17124-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. 22, 2020, 9:08 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.

Signed-off-by: Salvador Fandino <salvador@qindel.com>
---
 util/log.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

Comments

Stefan Hajnoczi Jan. 23, 2020, 11:32 a.m. UTC | #1 
On Wed, Jan 22, 2020 at 10:08:12PM +0100, salvador@qindel.com wrote:

Existing callers like vl.c:main() do:

   if (log_file) {
        qemu_set_log_filename(log_file, &error_fatal);
   }

Please fix up existing callers.  They won't need to check for NULL
anymore before calling qemu_set_log_filename().

> +    /* else, let logfilename be NULL indicating we want to use stderr */

Please update the doc comment instead.  That way callers know that
passing NULL is allowed without reading the implementation.
diff mbox series

Patch

diff --git a/util/log.c b/util/log.c
index 867264da8d..3cd2ebfdf4 100644
--- a/util/log.c
+++ b/util/log.c
@@ -151,22 +151,25 @@  void qemu_log_needs_buffers(void)
  */
 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);
+            }
     }
+    /* else, let logfilename be NULL indicating we want to use stderr */
+
     qemu_log_close();
     qemu_set_log(qemu_loglevel);
 }