Re: [PULL 5/5] qemu_set_log_filename: filename argument may be NULL
On 2/12/20 2:45 AM, Alex Bennée wrote: > > Richard Henderson writes: > >> On 1/30/20 1:38 PM, Stefan Hajnoczi wrote: >>> From: Salvador Fandino >>> >>> 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 >>> Message-Id: <20200123193626.19956-1-salva...@qindel.com> >>> Signed-off-by: Stefan Hajnoczi >>> --- >>> trace/control.c | 4 +--- >>> util/log.c | 28 >>> vl.c| 5 + >>> 3 files changed, 18 insertions(+), 19 deletions(-) >> >> This patch has broken -D for *-linux-user. >> After e144a605a, all logging goes to stderr. > > I posted: > > Subject: [PATCH] tracing: only allow -trace to override -D if set > Date: Tue, 11 Feb 2020 11:10:54 + > Message-Id: <2020021054.27538-1-alex.ben...@linaro.org> > > as a fix which partially reverted this. Thanks, that fixes it for me. r~
Re: [PULL 5/5] qemu_set_log_filename: filename argument may be NULL
Richard Henderson writes: > On 1/30/20 1:38 PM, Stefan Hajnoczi wrote: >> From: Salvador Fandino >> >> 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 >> Message-Id: <20200123193626.19956-1-salva...@qindel.com> >> Signed-off-by: Stefan Hajnoczi >> --- >> trace/control.c | 4 +--- >> util/log.c | 28 >> vl.c| 5 + >> 3 files changed, 18 insertions(+), 19 deletions(-) > > This patch has broken -D for *-linux-user. > After e144a605a, all logging goes to stderr. I posted: Subject: [PATCH] tracing: only allow -trace to override -D if set Date: Tue, 11 Feb 2020 11:10:54 + Message-Id: <2020021054.27538-1-alex.ben...@linaro.org> as a fix which partially reverted this. > >> +if (filename) { >> +char *pidstr = strstr(filename, "%"); >> +if (pidstr) { > > Also, the indentation is off. > > > r~ -- Alex Bennée
Re: [PULL 5/5] qemu_set_log_filename: filename argument may be NULL
On 1/30/20 1:38 PM, Stefan Hajnoczi wrote: > From: Salvador Fandino > > 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 > Message-Id: <20200123193626.19956-1-salva...@qindel.com> > Signed-off-by: Stefan Hajnoczi > --- > trace/control.c | 4 +--- > util/log.c | 28 > vl.c| 5 + > 3 files changed, 18 insertions(+), 19 deletions(-) This patch has broken -D for *-linux-user. After e144a605a, all logging goes to stderr. > +if (filename) { > +char *pidstr = strstr(filename, "%"); > +if (pidstr) { Also, the indentation is off. r~
[PULL 5/5] qemu_set_log_filename: filename argument may be NULL
From: Salvador Fandino 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 Message-Id: <20200123193626.19956-1-salva...@qindel.com> Signed-off-by: Stefan Hajnoczi --- trace/control.c | 4 +--- util/log.c | 28 vl.c| 5 + 3 files changed, 18 insertions(+), 19 deletions(-) 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, _fatal); -} +qemu_set_log_filename(file, _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 24951b51a9..7dcb0879c4 100644 --- a/vl.c +++ b/vl.c @@ -3903,10 +3903,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, _fatal); -} - +qemu_set_log_filename(log_file, _fatal); if (log_mask) { int mask; mask = qemu_str_to_log_mask(log_mask); -- 2.24.1