Patchwork qemu-option: Include name of invalid parameter in error message

login
register
mail settings
Submitter Stefan Weil
Date Aug. 1, 2010, 10:23 a.m.
Message ID <1280658182-4165-1-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/60470/
State New
Headers show

Comments

Stefan Weil - Aug. 1, 2010, 10:23 a.m.
All other error messages in qemu-option.c display the name
of the invalid parameter. This seems to be reasonable for
invalid identifiers, too. Without it, a debugger is needed
to find the name.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 qemu-option.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Markus Armbruster - Aug. 2, 2010, 8:40 a.m.
Stefan Weil <weil@mail.berlios.de> writes:

> All other error messages in qemu-option.c display the name
> of the invalid parameter. This seems to be reasonable for
> invalid identifiers, too. Without it, a debugger is needed
> to find the name.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  qemu-option.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index 1f8f41a..ccea267 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -694,7 +694,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exist
>  
>      if (id) {
>          if (!id_wellformed(id)) {
> -            qerror_report(QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, id, "an identifier");
>              error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n");
>              return NULL;
>          }

No.

QERR_INVALID_PARAMETER_VALUE's first argument is the parameter *name*,
not the parameter *value*.

In this case, the parameter name is "id".  The variable id contains the
parameter value.

If you need a debugger to find the offending id=, then location
information is lacking.  Could you give an example where it's hard to
find the offending parameter?

Here's an example where it's easy to find:

    $ qemu-system-x86_64 -device e1000,id=.
    qemu-system-x86_64: -device e1000,id=.: Parameter 'id' expects an identifier
    Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.

Patch

diff --git a/qemu-option.c b/qemu-option.c
index 1f8f41a..ccea267 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -694,7 +694,7 @@  QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exist
 
     if (id) {
         if (!id_wellformed(id)) {
-            qerror_report(QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, id, "an identifier");
             error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n");
             return NULL;
         }