diff mbox

[v2,10/16] vl.c: Convert error sentences to simpler phrases

Message ID 1446057425-16891-11-git-send-email-ehabkost@redhat.com
State New
Headers show

Commit Message

Eduardo Habkost Oct. 28, 2015, 6:36 p.m. UTC
Simplify some error messages by making them simple phrases instead of
full sentences.

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Markus Armbruster Oct. 29, 2015, 5:25 p.m. UTC | #1
Eduardo Habkost <ehabkost@redhat.com> writes:

> Simplify some error messages by making them simple phrases instead of
> full sentences.
>
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  vl.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 67147e0..b8c6c3c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1066,7 +1066,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>      fd_opaque = qemu_opt_get(opts, "opaque");
>  
>      if (fd < 0) {
> -        error_report("fd option is required and must be non-negative");
> +        error_report("non-negative 'fd' option required");
>          return -1;
>      }
>  

We need to make up our mind how to call a QemuOpts option's key=value
part.  qemu-options.c (actually qerror.h, which I still haven't killed
off) calls it "parameter", to avoid confusion with the entire option,
i.e. the -opt key=value,...

The error reporting lazy here: you get the same message for a missing fd
as for a negative fd, because the programmer couldn't be bothered to
detect the difference.  Not this patch's problem.

> @@ -1081,12 +1081,12 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>       */
>      flags = fcntl(fd, F_GETFD);
>      if (flags == -1 || (flags & FD_CLOEXEC)) {
> -        error_report("fd is not valid or already in use");
> +        error_report("fd not valid or already in use");
>          return -1;
>      }

As in PATCH 07, I'd prefer the phrasing with the verb.  More of the same
below.

Of course, the error message is crap either way.  "The order you gave is
invalid (but I'm not telling you why), or it clashes with some other
order I got (but I'm not telling you which one).  Pffft.  Not this
patch's problem.

>  
>      if (fdset_id < 0) {
> -        error_report("set option is required and must be non-negative");
> +        error_report("non-negative 'set' option required");
>          return -1;
>      }
>  

Lazy again.

> @@ -3751,7 +3751,7 @@ int main(int argc, char **argv, char **envp)
>                  option_rom[nb_option_roms].bootindex =
>                      qemu_opt_get_number(opts, "bootindex", -1);
>                  if (!option_rom[nb_option_roms].name) {
> -                    error_report("Option ROM file is not specified");
> +                    error_report("option ROM file not specified");
>                      exit(1);
>                  }
>                  nb_option_roms++;
> @@ -4225,11 +4225,11 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
> -        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
> +        error_report("-no-frame, -alt-grab and -ctrl-grab only valid "
>                       "for SDL, ignoring option");
>      }
>      if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
> -        error_report("-no-quit is only valid for GTK and SDL, "
> +        error_report("-no-quit only valid for GTK and SDL, "
>                       "ignoring option");
>      }
>  
> @@ -4373,7 +4373,7 @@ int main(int argc, char **argv, char **envp)
>      cpu_ticks_init();
>      if (icount_opts) {
>          if (kvm_enabled() || xen_enabled()) {
> -            error_report("-icount is not allowed with kvm or xen");
> +            error_report("-icount not allowed with kvm or xen");
>              exit(1);
>          }
>          configure_icount(icount_opts, &error_abort);
diff mbox

Patch

diff --git a/vl.c b/vl.c
index 67147e0..b8c6c3c 100644
--- a/vl.c
+++ b/vl.c
@@ -1066,7 +1066,7 @@  static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
     fd_opaque = qemu_opt_get(opts, "opaque");
 
     if (fd < 0) {
-        error_report("fd option is required and must be non-negative");
+        error_report("non-negative 'fd' option required");
         return -1;
     }
 
@@ -1081,12 +1081,12 @@  static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
      */
     flags = fcntl(fd, F_GETFD);
     if (flags == -1 || (flags & FD_CLOEXEC)) {
-        error_report("fd is not valid or already in use");
+        error_report("fd not valid or already in use");
         return -1;
     }
 
     if (fdset_id < 0) {
-        error_report("set option is required and must be non-negative");
+        error_report("non-negative 'set' option required");
         return -1;
     }
 
@@ -3751,7 +3751,7 @@  int main(int argc, char **argv, char **envp)
                 option_rom[nb_option_roms].bootindex =
                     qemu_opt_get_number(opts, "bootindex", -1);
                 if (!option_rom[nb_option_roms].name) {
-                    error_report("Option ROM file is not specified");
+                    error_report("option ROM file not specified");
                     exit(1);
                 }
                 nb_option_roms++;
@@ -4225,11 +4225,11 @@  int main(int argc, char **argv, char **envp)
     }
 
     if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
-        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
+        error_report("-no-frame, -alt-grab and -ctrl-grab only valid "
                      "for SDL, ignoring option");
     }
     if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
-        error_report("-no-quit is only valid for GTK and SDL, "
+        error_report("-no-quit only valid for GTK and SDL, "
                      "ignoring option");
     }
 
@@ -4373,7 +4373,7 @@  int main(int argc, char **argv, char **envp)
     cpu_ticks_init();
     if (icount_opts) {
         if (kvm_enabled() || xen_enabled()) {
-            error_report("-icount is not allowed with kvm or xen");
+            error_report("-icount not allowed with kvm or xen");
             exit(1);
         }
         configure_icount(icount_opts, &error_abort);