* [PATCH] qemu_set_log_filename: filename argument may be NULL
@ 2020-01-22 20:21 salvador
0 siblings, 0 replies; 6+ messages in thread
From: salvador @ 2020-01-22 20:21 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, sfandino, Salvador Fandino
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(-)
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);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] qemu_set_log_filename: filename argument may be NULL
2020-01-23 19:36 ` salvador
2020-01-24 10:23 ` Stefan Hajnoczi
@ 2020-01-24 10:24 ` Stefan Hajnoczi
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2020-01-24 10:24 UTC (permalink / raw)
To: salvador; +Cc: qemu-trivial, sfandino, Paolo Bonzini, qemu-devel, stefanha
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] qemu_set_log_filename: filename argument may be NULL
2020-01-23 19:36 ` salvador
@ 2020-01-24 10:23 ` Stefan Hajnoczi
2020-01-24 10:24 ` Stefan Hajnoczi
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2020-01-24 10:23 UTC (permalink / raw)
To: salvador; +Cc: qemu-trivial, sfandino, Paolo Bonzini, qemu-devel, stefanha
[-- Attachment #1: Type: text/plain, Size: 685 bytes --]
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>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] qemu_set_log_filename: filename argument may be NULL
2020-01-23 11:32 ` Stefan Hajnoczi
@ 2020-01-23 19:36 ` salvador
2020-01-24 10:23 ` Stefan Hajnoczi
2020-01-24 10:24 ` Stefan Hajnoczi
0 siblings, 2 replies; 6+ messages in thread
From: salvador @ 2020-01-23 19:36 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-trivial, stefanha, sfandino, Salvador Fandino,
Stefan Hajnoczi, Paolo Bonzini
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(-)
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);
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] qemu_set_log_filename: filename argument may be NULL
2020-01-22 21:08 salvador
@ 2020-01-23 11:32 ` Stefan Hajnoczi
2020-01-23 19:36 ` salvador
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2020-01-23 11:32 UTC (permalink / raw)
To: salvador; +Cc: qemu-trivial, sfandino, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 509 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] qemu_set_log_filename: filename argument may be NULL
@ 2020-01-22 21:08 salvador
2020-01-23 11:32 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: salvador @ 2020-01-22 21:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, sfandino, Salvador Fandino
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(-)
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);
}
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-24 10:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-22 20:21 [PATCH] qemu_set_log_filename: filename argument may be NULL salvador
2020-01-22 21:08 salvador
2020-01-23 11:32 ` Stefan Hajnoczi
2020-01-23 19:36 ` salvador
2020-01-24 10:23 ` Stefan Hajnoczi
2020-01-24 10:24 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.