* [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup
@ 2015-01-26 11:12 Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 1/7] cpu-exec: drop dead assignment Paolo Bonzini
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
Patch 2 is the cleanup. The other six patches make the tcg, utils and
migration components clean.
Paolo Bonzini (7):
cpu-exec: drop dead assignment
cpu-exec: simplify icount code
uri: avoid NULL arguments to strcmp
qemu-sockets: improve error reporting in unix_listen_opts
cutils: refine strtol error handling in parse_debug_env
aes: remove a dead return statement
migration: do floating-point division
cpu-exec.c | 12 +++---------
migration/migration.c | 2 +-
util/aes.c | 2 +-
util/cutils.c | 4 ++--
util/qemu-sockets.c | 24 ++++++++++++++++++------
util/uri.c | 4 +++-
6 files changed, 28 insertions(+), 20 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/7] cpu-exec: drop dead assignment
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 2/7] cpu-exec: simplify icount code Paolo Bonzini
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
All uses of TB inside cpu_exec are dominated by "tb = tb_find_fast(env)",
and there are no uses after the switch statement. So the assignment
is dead, as reported by Coverity.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpu-exec.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index a4f0eff..7aab86d 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -497,7 +497,6 @@ int cpu_exec(CPUArchState *env)
* interrupt_request) which we will handle
* next time around the loop.
*/
- tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
next_tb = 0;
break;
case TB_EXIT_ICOUNT_EXPIRED:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/7] cpu-exec: simplify icount code
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 1/7] cpu-exec: drop dead assignment Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 3/7] uri: avoid NULL arguments to strcmp Paolo Bonzini
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
Use MIN instead of an "if" statement. Move "tb" assignment where
the value is actually used.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpu-exec.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index 7aab86d..af56a34 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -502,22 +502,17 @@ int cpu_exec(CPUArchState *env)
case TB_EXIT_ICOUNT_EXPIRED:
{
/* Instruction counter expired. */
- int insns_left;
- tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
- insns_left = cpu->icount_decr.u32;
+ int insns_left = cpu->icount_decr.u32;
if (cpu->icount_extra && insns_left >= 0) {
/* Refill decrementer and continue execution. */
cpu->icount_extra += insns_left;
- if (cpu->icount_extra > 0xffff) {
- insns_left = 0xffff;
- } else {
- insns_left = cpu->icount_extra;
- }
+ insns_left = MIN(0xffff, cpu->icount_extra);
cpu->icount_extra -= insns_left;
cpu->icount_decr.u16.low = insns_left;
} else {
if (insns_left > 0) {
/* Execute remaining instructions. */
+ tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
cpu_exec_nocache(env, insns_left, tb);
align_clocks(&sc, cpu);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/7] uri: avoid NULL arguments to strcmp
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 1/7] cpu-exec: drop dead assignment Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 2/7] cpu-exec: simplify icount code Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 4/7] qemu-sockets: improve error reporting in unix_listen_opts Paolo Bonzini
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
Detected by Coverity.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/uri.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/util/uri.c b/util/uri.c
index 918d235..014d40d 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1971,7 +1971,9 @@ uri_resolve_relative (const char *uri, const char * base)
val = g_strdup (uri);
goto done;
}
- if (!strcmp(bas->path, ref->path)) {
+ if (bas->path == ref->path ||
+ (bas->path != NULL && ref->path != NULL &&
+ !strcmp(bas->path, ref->path))) {
val = g_strdup("");
goto done;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/7] qemu-sockets: improve error reporting in unix_listen_opts
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
` (2 preceding siblings ...)
2015-01-26 11:12 ` [Qemu-devel] [PATCH 3/7] uri: avoid NULL arguments to strcmp Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env Paolo Bonzini
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
Coverity complains about not checking the returned value of mkstemp. While
at it, also improve error checking for snprintf, and refine error messages
in general.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-sockets.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index a76bb3c..cf4b91f 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -694,7 +694,7 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
- error_setg_errno(errp, errno, "Failed to create socket");
+ error_setg_errno(errp, errno, "Failed to create Unix socket");
return -1;
}
@@ -703,9 +703,15 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
if (path && strlen(path)) {
snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
} else {
- char *tmpdir = getenv("TMPDIR");
- snprintf(un.sun_path, sizeof(un.sun_path), "%s/qemu-socket-XXXXXX",
- tmpdir ? tmpdir : "/tmp");
+ const char *tmpdir = getenv("TMPDIR");
+ tmpdir = tmpdir ? tmpdir : "/tmp";
+ if (snprintf(un.sun_path, sizeof(un.sun_path), "%s/qemu-socket-XXXXXX",
+ tmpdir) >= sizeof(un.sun_path)) {
+ error_setg_errno(errp, errno,
+ "TMPDIR environment variable (%s) too large", tmpdir);
+ goto err;
+ }
+
/*
* This dummy fd usage silences the mktemp() unsecure warning.
* Using mkstemp() doesn't make things more secure here
@@ -713,13 +719,19 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
* to unlink first and thus re-open the race window. The
* worst case possible is bind() failing, i.e. a DoS attack.
*/
- fd = mkstemp(un.sun_path); close(fd);
+ fd = mkstemp(un.sun_path);
+ if (fd < 0) {
+ error_setg_errno(errp, errno,
+ "Failed to make a temporary socket name in %s", tmpdir);
+ goto err;
+ }
+ close(fd);
qemu_opt_set(opts, "path", un.sun_path);
}
unlink(un.sun_path);
if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
- error_setg_errno(errp, errno, "Failed to bind socket");
+ error_setg_errno(errp, errno, "Failed to bind socket to %s", un.sun_path);
goto err;
}
if (listen(sock, 1) < 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
` (3 preceding siblings ...)
2015-01-26 11:12 ` [Qemu-devel] [PATCH 4/7] qemu-sockets: improve error reporting in unix_listen_opts Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-02-07 8:52 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2015-01-26 11:12 ` [Qemu-devel] [PATCH 6/7] aes: remove a dead return statement Paolo Bonzini
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
Avoid truncation of a 64-bit long to a 32-bit int, and check for errno
(especially ERANGE).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/cutils.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/cutils.c b/util/cutils.c
index dbe7412..f227064 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -523,7 +523,7 @@ int parse_debug_env(const char *name, int max, int initial)
{
char *debug_env = getenv(name);
char *inv = NULL;
- int debug;
+ long debug;
if (!debug_env) {
return initial;
@@ -532,7 +532,7 @@ int parse_debug_env(const char *name, int max, int initial)
if (inv == debug_env) {
return initial;
}
- if (debug < 0 || debug > max) {
+ if (debug < 0 || debug > max || errno != 0) {
fprintf(stderr, "warning: %s not in [0, %d]", name, max);
return initial;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 6/7] aes: remove a dead return statement
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
` (4 preceding siblings ...)
2015-01-26 11:12 ` [Qemu-devel] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 7/7] migration: do floating-point division Paolo Bonzini
2015-02-07 9:00 ` [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup Michael Tokarev
7 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
bits is checked to be 128, 192 or 256 at the beginning of the function.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/aes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/aes.c b/util/aes.c
index 6058f19..f42126a 100644
--- a/util/aes.c
+++ b/util/aes.c
@@ -1161,7 +1161,7 @@ int AES_set_encrypt_key(const unsigned char *userKey, const int bits,
rk += 8;
}
}
- return 0;
+ abort();
}
/**
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 7/7] migration: do floating-point division
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
` (5 preceding siblings ...)
2015-01-26 11:12 ` [Qemu-devel] [PATCH 6/7] aes: remove a dead return statement Paolo Bonzini
@ 2015-01-26 11:12 ` Paolo Bonzini
2015-01-26 18:06 ` Dr. David Alan Gilbert
2015-02-07 9:00 ` [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup Michael Tokarev
7 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2015-01-26 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, armbru
Dividing integer expressions transferred_bytes and time_spent, and then converting
the integer quotient to type double. Any remainder, or fractional part of the
quotient, is ignored. Fix this.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
migration/migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..6db75b8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -646,7 +646,7 @@ static void *migration_thread(void *opaque)
if (current_time >= initial_time + BUFFER_DELAY) {
uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
uint64_t time_spent = current_time - initial_time;
- double bandwidth = transferred_bytes / time_spent;
+ double bandwidth = (double)transferred_bytes / time_spent;
max_size = bandwidth * migrate_max_downtime() / 1000000;
s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] migration: do floating-point division
2015-01-26 11:12 ` [Qemu-devel] [PATCH 7/7] migration: do floating-point division Paolo Bonzini
@ 2015-01-26 18:06 ` Dr. David Alan Gilbert
2015-01-26 18:15 ` Juan Quintela
0 siblings, 1 reply; 15+ messages in thread
From: Dr. David Alan Gilbert @ 2015-01-26 18:06 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-trivial, quintela, qemu-devel, armbru
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Dividing integer expressions transferred_bytes and time_spent, and then converting
> the integer quotient to type double. Any remainder, or fractional part of the
> quotient, is ignored. Fix this.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> migration/migration.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b3adbc6..6db75b8 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -646,7 +646,7 @@ static void *migration_thread(void *opaque)
> if (current_time >= initial_time + BUFFER_DELAY) {
> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
> uint64_t time_spent = current_time - initial_time;
> - double bandwidth = transferred_bytes / time_spent;
> + double bandwidth = (double)transferred_bytes / time_spent;
> max_size = bandwidth * migrate_max_downtime() / 1000000;
>
> s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
This feels like it would be better to fix this by merging it into
the s->mbps calculation just off the bottom; we currently have:
uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
uint64_t time_spent = current_time - initial_time;
double bandwidth = transferred_bytes / time_spent;
max_size = bandwidth * migrate_max_downtime() / 1000000;
s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
Note that the mbps has a check for time_spent being 0 - if that can ever happen,
how come 'bandwidth' has never triggered it?
transferred_bytes - bytes
time_spent - ms
bandwidth - bytes/ms
migrate_max_downtime - in ns
s->mbps - mbit/s
giving
max_size = bytes/ms * time-in-ns / 1000000
= bytes/ms * time-in-ms
= bytes
so how about something like:
uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
uint64_t time_spent = current_time - initial_time;
double bytes_s = (double) transferred_bytes / ((double) time_spent / 1000.0));
s->mbps = (bytes_s * 8.0) / 1000000.0;
max_size = bytes_s * (migrate_max_downtime() / 1000000000.0);
that also needs the trace fixing and the line a few lines below, I *think* we have
dirty_bytes_rate is in bytes/second ? (arch_init.c)
expected_downtime in ms ?
s->expected_downtime = s->dirty_bytes_rate / bandwidth;
so, bytes/s / bytes/ms erm that's supposed to come out as time in ms
s->expected_downtime = (int64_t)(1000 * (double)s->dirty_bytes_rate / bytes_s);
Yeuch.
Dave
> --
> 1.8.3.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] migration: do floating-point division
2015-01-26 18:06 ` Dr. David Alan Gilbert
@ 2015-01-26 18:15 ` Juan Quintela
0 siblings, 0 replies; 15+ messages in thread
From: Juan Quintela @ 2015-01-26 18:15 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: qemu-trivial, Paolo Bonzini, qemu-devel, armbru
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> Dividing integer expressions transferred_bytes and time_spent, and then converting
>> the integer quotient to type double. Any remainder, or fractional part of the
>> quotient, is ignored. Fix this.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> migration/migration.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index b3adbc6..6db75b8 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -646,7 +646,7 @@ static void *migration_thread(void *opaque)
>> if (current_time >= initial_time + BUFFER_DELAY) {
>> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
>> uint64_t time_spent = current_time - initial_time;
>> - double bandwidth = transferred_bytes / time_spent;
>> + double bandwidth = (double)transferred_bytes / time_spent;
>> max_size = bandwidth * migrate_max_downtime() / 1000000;
>>
>> s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
Reviewed-by: Juan Quintela <quintela@redhat.com>
we had that fix on RHEL, but I forgot to push it upstream (it was not
needed as it optimized the calculations at the time).
>
> This feels like it would be better to fix this by merging it into
> the s->mbps calculation just off the bottom; we currently have:
>
> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
> uint64_t time_spent = current_time - initial_time;
> double bandwidth = transferred_bytes / time_spent;
> max_size = bandwidth * migrate_max_downtime() / 1000000;
>
> s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
> ((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
>
>
> Note that the mbps has a check for time_spent being 0 - if that can ever happen,
> how come 'bandwidth' has never triggered it?
>
> transferred_bytes - bytes
> time_spent - ms
> bandwidth - bytes/ms
> migrate_max_downtime - in ns
> s->mbps - mbit/s
>
> giving
>
> max_size = bytes/ms * time-in-ns / 1000000
> = bytes/ms * time-in-ms
> = bytes
>
> so how about something like:
> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
> uint64_t time_spent = current_time - initial_time;
> double bytes_s = (double) transferred_bytes / ((double) time_spent / 1000.0));
> s->mbps = (bytes_s * 8.0) / 1000000.0;
> max_size = bytes_s * (migrate_max_downtime() / 1000000000.0);
I think we can do something like this on the normal tree, or just go for
sane units left and right?
> that also needs the trace fixing and the line a few lines below, I *think* we have
> dirty_bytes_rate is in bytes/second ? (arch_init.c)
> expected_downtime in ms ?
> s->expected_downtime = s->dirty_bytes_rate / bandwidth;
> so, bytes/s / bytes/ms erm that's supposed to come out as time in ms
>
> s->expected_downtime = (int64_t)(1000 * (double)s->dirty_bytes_rate / bytes_s);
>
> Yeuch.
>
> Dave
>
>> --
>> 1.8.3.1
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env
2015-01-26 11:12 ` [Qemu-devel] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env Paolo Bonzini
@ 2015-02-07 8:52 ` Michael Tokarev
0 siblings, 0 replies; 15+ messages in thread
From: Michael Tokarev @ 2015-02-07 8:52 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, armbru
26.01.2015 14:12, Paolo Bonzini wrote:
> Avoid truncation of a 64-bit long to a 32-bit int, and check for errno
> (especially ERANGE).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> util/cutils.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/cutils.c b/util/cutils.c
> index dbe7412..f227064 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -523,7 +523,7 @@ int parse_debug_env(const char *name, int max, int initial)
> {
> char *debug_env = getenv(name);
> char *inv = NULL;
> - int debug;
> + long debug;
>
> if (!debug_env) {
> return initial;
> @@ -532,7 +532,7 @@ int parse_debug_env(const char *name, int max, int initial)
> if (inv == debug_env) {
> return initial;
> }
> - if (debug < 0 || debug > max) {
> + if (debug < 0 || debug > max || errno != 0) {
It is not really right to check errno without (re)setting it
before call to strtol().
Thanks,
/mjt
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
` (6 preceding siblings ...)
2015-01-26 11:12 ` [Qemu-devel] [PATCH 7/7] migration: do floating-point division Paolo Bonzini
@ 2015-02-07 9:00 ` Michael Tokarev
2015-02-07 20:01 ` Paolo Bonzini
7 siblings, 1 reply; 15+ messages in thread
From: Michael Tokarev @ 2015-02-07 9:00 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, armbru
26.01.2015 14:12, Paolo Bonzini wrote:
> Patch 2 is the cleanup. The other six patches make the tcg, utils and
> migration components clean.
Applied to -trivial patches 1,2,3,4 and 6, not applied 5 (due to
questionable errno checking addition) and 7, due to a discussion.
Thanks,
/mjt
>
> Paolo Bonzini (7):
> cpu-exec: drop dead assignment
> cpu-exec: simplify icount code
> uri: avoid NULL arguments to strcmp
> qemu-sockets: improve error reporting in unix_listen_opts
> cutils: refine strtol error handling in parse_debug_env
> aes: remove a dead return statement
> migration: do floating-point division
>
> cpu-exec.c | 12 +++---------
> migration/migration.c | 2 +-
> util/aes.c | 2 +-
> util/cutils.c | 4 ++--
> util/qemu-sockets.c | 24 ++++++++++++++++++------
> util/uri.c | 4 +++-
> 6 files changed, 28 insertions(+), 20 deletions(-)
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup
2015-02-07 9:00 ` [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup Michael Tokarev
@ 2015-02-07 20:01 ` Paolo Bonzini
2015-02-07 20:39 ` Michael Tokarev
2015-02-09 12:17 ` Juan Quintela
0 siblings, 2 replies; 15+ messages in thread
From: Paolo Bonzini @ 2015-02-07 20:01 UTC (permalink / raw)
To: Michael Tokarev, qemu-devel
Cc: qemu-trivial, Amit Shah, armbru, Juan Quintela
On 07/02/2015 10:00, Michael Tokarev wrote:
>> > Patch 2 is the cleanup. The other six patches make the tcg, utils and
>> > migration components clean.
> Applied to -trivial patches 1,2,3,4 and 6, not applied 5 (due to
> questionable errno checking addition) and 7, due to a discussion.
Thanks, will resubmit 5.
Juan/Amit, can you pick up 7 which Juan has already reviewed?
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup
2015-02-07 20:01 ` Paolo Bonzini
@ 2015-02-07 20:39 ` Michael Tokarev
2015-02-09 12:17 ` Juan Quintela
1 sibling, 0 replies; 15+ messages in thread
From: Michael Tokarev @ 2015-02-07 20:39 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, Amit Shah, armbru, Juan Quintela
07.02.2015 23:01, Paolo Bonzini wrote:
> On 07/02/2015 10:00, Michael Tokarev wrote:
>>>> Patch 2 is the cleanup. The other six patches make the tcg, utils and
>>>> migration components clean.
>> Applied to -trivial patches 1,2,3,4 and 6, not applied 5 (due to
>> questionable errno checking addition) and 7, due to a discussion.
>
> Thanks, will resubmit 5.
>
> Juan/Amit, can you pick up 7 which Juan has already reviewed?
I've no prob with 7, but please let's agree which way we're taking
there -- whenever subsequent fixes are needed, should be made on
the top of this change, or whole thing should be done differently.
Thanks.
/mjt
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup
2015-02-07 20:01 ` Paolo Bonzini
2015-02-07 20:39 ` Michael Tokarev
@ 2015-02-09 12:17 ` Juan Quintela
1 sibling, 0 replies; 15+ messages in thread
From: Juan Quintela @ 2015-02-09 12:17 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-trivial, Amit Shah, Michael Tokarev, qemu-devel, armbru
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 07/02/2015 10:00, Michael Tokarev wrote:
>>> > Patch 2 is the cleanup. The other six patches make the tcg, utils and
>>> > migration components clean.
>> Applied to -trivial patches 1,2,3,4 and 6, not applied 5 (due to
>> questionable errno checking addition) and 7, due to a discussion.
>
> Thanks, will resubmit 5.
>
> Juan/Amit, can you pick up 7 which Juan has already reviewed?
I pick 7.
Thanks, Juan.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-02-09 12:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-26 11:12 [Qemu-devel] [PATCH 0/7] Six coverity fixes and a cleanup Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 1/7] cpu-exec: drop dead assignment Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 2/7] cpu-exec: simplify icount code Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 3/7] uri: avoid NULL arguments to strcmp Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 4/7] qemu-sockets: improve error reporting in unix_listen_opts Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 5/7] cutils: refine strtol error handling in parse_debug_env Paolo Bonzini
2015-02-07 8:52 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2015-01-26 11:12 ` [Qemu-devel] [PATCH 6/7] aes: remove a dead return statement Paolo Bonzini
2015-01-26 11:12 ` [Qemu-devel] [PATCH 7/7] migration: do floating-point division Paolo Bonzini
2015-01-26 18:06 ` Dr. David Alan Gilbert
2015-01-26 18:15 ` Juan Quintela
2015-02-07 9:00 ` [Qemu-devel] [Qemu-trivial] [PATCH 0/7] Six coverity fixes and a cleanup Michael Tokarev
2015-02-07 20:01 ` Paolo Bonzini
2015-02-07 20:39 ` Michael Tokarev
2015-02-09 12:17 ` Juan Quintela
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.