diff mbox

memsave: Improve and disambiguate error message

Message ID 20150208121438.GA6290@pd.tnic
State New
Headers show

Commit Message

Borislav Petkov Feb. 8, 2015, 12:14 p.m. UTC
On Sun, Feb 08, 2015 at 11:09:24AM +0100, Paolo Bonzini wrote:
> Cc: qemu-trivial@nongnu.org

Thanks.

But, there's more b0rked with this error message so you might wanna take
the patch below instead.

Btw, what are the vim settings when doing patches for qemu, this must be
documented somewhere as coding style differs significantly from that of
the kernel?

:-)

---
From: Borislav Petkov <bp@suse.de>
Subject: [PATCH] memsave: Improve and disambiguate error message

When requesting a size which cannot be read, the error message shows
a different address which is misleading to the user and it looks like
something's wrong with the address parsing. This is because the input
@addr variable is incremented in the memory dumping loop:

(qemu) memsave 0xffffffff8418069c 0xb00000 mem
Invalid addr 0xffffffff849ffe9c specified

Fix that by saving the original address and size and use them in the
error message:

(qemu) memsave 0xffffffff8418069c 0xb00000 mem
Invalid addr 0xffffffff8418069c/size 11534336 specified

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 cpus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini Feb. 8, 2015, 9:46 p.m. UTC | #1
On 08/02/2015 13:14, Borislav Petkov wrote:
> On Sun, Feb 08, 2015 at 11:09:24AM +0100, Paolo Bonzini wrote:
>> Cc: qemu-trivial@nongnu.org
> 
> Thanks.
> 
> But, there's more b0rked with this error message so you might wanna take
> the patch below instead.
> 
> Btw, what are the vim settings when doing patches for qemu, this must be
> documented somewhere as coding style differs significantly from that of
> the kernel?

I use ":set shiftwidth=4 expandtab" and c-indent mode.

Quick googling for "vim four spaces" suggests the following alternatives:

* ":set tabstop=8 expandtab shiftwidth=4 softtabstop=4"

* "set tabstop=8 softtabstop=0 expandtab shiftwidth=4 smarttab".

Paolo
Borislav Petkov Feb. 10, 2015, 9:22 a.m. UTC | #2
On Sun, Feb 08, 2015 at 10:46:05PM +0100, Paolo Bonzini wrote:
> I use ":set shiftwidth=4 expandtab" and c-indent mode.
> 
> Quick googling for "vim four spaces" suggests the following alternatives:
> 
> * ":set tabstop=8 expandtab shiftwidth=4 softtabstop=4"
> 
> * "set tabstop=8 softtabstop=0 expandtab shiftwidth=4 smarttab".

Thanks Paolo!

:-)
Michael Tokarev Feb. 10, 2015, 7:06 p.m. UTC | #3
08.02.2015 15:14, Borislav Petkov wrote:
> On Sun, Feb 08, 2015 at 11:09:24AM +0100, Paolo Bonzini wrote:
>> Cc: qemu-trivial@nongnu.org
> 
> Thanks.
> 
> But, there's more b0rked with this error message so you might wanna take
> the patch below instead.
> 
> Btw, what are the vim settings when doing patches for qemu, this must be
> documented somewhere as coding style differs significantly from that of
> the kernel?
> 
> :-)
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Subject: [PATCH] memsave: Improve and disambiguate error message
> 
> When requesting a size which cannot be read, the error message shows
> a different address which is misleading to the user and it looks like
> something's wrong with the address parsing. This is because the input
> @addr variable is incremented in the memory dumping loop:

Applied this version to -trivial, thank you!

/mjt


> (qemu) memsave 0xffffffff8418069c 0xb00000 mem
> Invalid addr 0xffffffff849ffe9c specified
> 
> Fix that by saving the original address and size and use them in the
> error message:
> 
> (qemu) memsave 0xffffffff8418069c 0xb00000 mem
> Invalid addr 0xffffffff8418069c/size 11534336 specified
> 
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  cpus.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 0cdd1d71560b..a5c7cad00fc9 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1466,6 +1466,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
>      uint32_t l;
>      CPUState *cpu;
>      uint8_t buf[1024];
> +    int64_t orig_addr = addr, orig_size = size;
>  
>      if (!has_cpu) {
>          cpu_index = 0;
> @@ -1489,7 +1490,8 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
>          if (l > size)
>              l = size;
>          if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
> -            error_setg(errp, "Invalid addr 0x%016" PRIx64 "specified", addr);
> +            error_setg(errp, "Invalid addr 0x%016" PRIx64 "/size %" PRId64
> +                             " specified", orig_addr, orig_size);
>              goto exit;
>          }
>          if (fwrite(buf, 1, l, f) != l) {
>
diff mbox

Patch

diff --git a/cpus.c b/cpus.c
index 0cdd1d71560b..a5c7cad00fc9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1466,6 +1466,7 @@  void qmp_memsave(int64_t addr, int64_t size, const char *filename,
     uint32_t l;
     CPUState *cpu;
     uint8_t buf[1024];
+    int64_t orig_addr = addr, orig_size = size;
 
     if (!has_cpu) {
         cpu_index = 0;
@@ -1489,7 +1490,8 @@  void qmp_memsave(int64_t addr, int64_t size, const char *filename,
         if (l > size)
             l = size;
         if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
-            error_setg(errp, "Invalid addr 0x%016" PRIx64 "specified", addr);
+            error_setg(errp, "Invalid addr 0x%016" PRIx64 "/size %" PRId64
+                             " specified", orig_addr, orig_size);
             goto exit;
         }
         if (fwrite(buf, 1, l, f) != l) {