qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] vl.c: Replace fprintf(stderr) with error_report


From: Andrew Jones
Subject: Re: [Qemu-devel] [PATCH] vl.c: Replace fprintf(stderr) with error_report()
Date: Tue, 27 Oct 2015 08:30:38 +0100
User-agent: Mutt/1.5.23.1 (2014-03-12)

On Mon, Oct 26, 2015 at 03:13:43PM -0200, Eduardo Habkost wrote:
> This replaces most fprintf(stderr) calls on vl.c with error_report().
> 
> The trailing newlines, "qemu:" and "error:" message prefixes were
> removed.
> 
> The only remaining fprintf(stderr) calls are the ones at
> qemu_kill_report(), because the error mesage is split in multiple
> fprintf() calls.
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> Not sure if this is appropriate post soft-freeze, but if we are going to apply
> the max-cpus patch from Drew before 2.5.0, we could simply change all the
> fprintf() calls in a single step.

In addition to Markus' and Eric's comments, I think we should

1. make sure the first word's case is correct. Lowercase for phrases,
   uppercase for sentences and proper nouns.
2. make sure the 'warning' prefix is consistent in all uses. This should
   be a QEMU-wide change. Maybe we need an error_report_warn variant?
3. make sure past tense is used in phrases like "failed to do..."
4. only break the line if necessary. Some of the lines are broken even
   though they could fit in 80 char. Probably the opposite exists too,
   i.e. some are > 80.
  
I've tried to point out a few of the cases below that inspired me to
to write these suggestions.

Thanks,
drew


> ---
>  vl.c | 228 
> +++++++++++++++++++++++++++++++++----------------------------------
>  1 file changed, 112 insertions(+), 116 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index dffaf09..db4a1f5 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -674,9 +674,9 @@ void runstate_set(RunState new_state)
>      assert(new_state < RUN_STATE_MAX);
>  
>      if (!runstate_valid_transitions[current_run_state][new_state]) {
> -        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
> -                RunState_lookup[current_run_state],
> -                RunState_lookup[new_state]);
> +        error_report("invalid runstate transition: '%s' -> '%s'",
> +                     RunState_lookup[current_run_state],
> +                     RunState_lookup[new_state]);
>          abort();
>      }
>      trace_runstate_set(new_state);
> @@ -828,8 +828,8 @@ static void configure_rtc_date_offset(const char 
> *startdate, int legacy)
>          rtc_start_date = mktimegm(&tm);
>          if (rtc_start_date == -1) {
>          date_fail:
> -            fprintf(stderr, "Invalid date format. Valid formats are:\n"
> -                            "'2006-06-17T16:01:21' or '2006-06-17'\n");
> +            error_report("Invalid date format. Valid formats are:\n"
> +                         "'2006-06-17T16:01:21' or '2006-06-17'");
>              exit(1);
>          }
>          rtc_date_offset = qemu_time() - rtc_start_date;
> @@ -859,7 +859,7 @@ static void configure_rtc(QemuOpts *opts)
>          } else if (!strcmp(value, "vm")) {
>              rtc_clock = QEMU_CLOCK_VIRTUAL;
>          } else {
> -            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
> +            error_report("invalid option value '%s'", value);
>              exit(1);
>          }
>      }
> @@ -879,7 +879,7 @@ static void configure_rtc(QemuOpts *opts)
>          } else if (!strcmp(value, "none")) {
>              /* discard is default */
>          } else {
> -            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
> +            error_report("invalid option value '%s'", value);
>              exit(1);
>          }
>      }
> @@ -905,7 +905,7 @@ static int bt_hci_parse(const char *str)
>      bdaddr_t bdaddr;
>  
>      if (nb_hcis >= MAX_NICS) {
> -        fprintf(stderr, "qemu: Too many bluetooth HCIs (max %i).\n", 
> MAX_NICS);
> +        error_report("Too many bluetooth HCIs (max %i).", MAX_NICS);
>          return -1;
>      }
>  
> @@ -931,8 +931,8 @@ static void bt_vhci_add(int vlan_id)
>      struct bt_scatternet_s *vlan = qemu_find_bt_vlan(vlan_id);
>  
>      if (!vlan->slave)
> -        fprintf(stderr, "qemu: warning: adding a VHCI to "
> -                        "an empty scatternet %i\n", vlan_id);
> +        error_report("warning: adding a VHCI to "
> +                     "an empty scatternet %i", vlan_id);
>  
>      bt_vhci_init(bt_new_hci(vlan));
>  }
> @@ -950,7 +950,7 @@ static struct bt_device_s *bt_device_add(const char *opt)
>      if (endp) {
>          vlan_id = strtol(endp + 6, &endp, 0);
>          if (*endp) {
> -            fprintf(stderr, "qemu: unrecognised bluetooth vlan Id\n");
> +            error_report("unrecognised bluetooth vlan Id");
>              return 0;
>          }
>      }
> @@ -958,13 +958,13 @@ static struct bt_device_s *bt_device_add(const char 
> *opt)
>      vlan = qemu_find_bt_vlan(vlan_id);
>  
>      if (!vlan->slave)
> -        fprintf(stderr, "qemu: warning: adding a slave device to "
> -                        "an empty scatternet %i\n", vlan_id);
> +        error_report("warning: adding a slave device to "
> +                     "an empty scatternet %i", vlan_id);
>  
>      if (!strcmp(devname, "keyboard"))
>          return bt_keyboard_init(vlan);
>  
> -    fprintf(stderr, "qemu: unsupported bluetooth device `%s'\n", devname);
> +    error_report("unsupported bluetooth device `%s'", devname);
>      return 0;
>  }
>  
> @@ -987,11 +987,11 @@ static int bt_parse(const char *opt)
>                  if (strstart(endp, ",vlan=", &p)) {
>                      vlan = strtol(p, (char **) &endp, 0);
>                      if (*endp) {
> -                        fprintf(stderr, "qemu: bad scatternet '%s'\n", p);
> +                        error_report("bad scatternet '%s'", p);
>                          return 1;
>                      }
>                  } else {
> -                    fprintf(stderr, "qemu: bad parameter '%s'\n", endp + 1);
> +                    error_report("bad parameter '%s'", endp + 1);
>                      return 1;
>                  }
>              } else
> @@ -1003,7 +1003,7 @@ static int bt_parse(const char *opt)
>      } else if (strstart(opt, "device:", &endp))
>          return !bt_device_add(endp);
>  
> -    fprintf(stderr, "qemu: bad bluetooth parameter '%s'\n", opt);
> +    error_report("bad bluetooth parameter '%s'", opt);
>      return 1;
>  }
>  
> @@ -1220,18 +1220,19 @@ static void smp_parse(QemuOpts *opts)
>          } else if (threads == 0) {
>              threads = cpus / (cores * sockets);
>          } else if (sockets * cores * threads < cpus) {
> -            fprintf(stderr, "cpu topology: error: "
> -                    "sockets (%u) * cores (%u) * threads (%u) < "
> -                    "smp_cpus (%u)\n",
> -                    sockets, cores, threads, cpus);
> +            error_report("cpu topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) < "
> +                         "smp_cpus (%u)",
> +                         sockets, cores, threads, cpus);
>              exit(1);
>          }
>  
>          max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
>          if (sockets * cores * threads > max_cpus) {
> -            fprintf(stderr, "cpu topology: error: "
> -                    "sockets (%u) * cores (%u) * threads (%u) > maxcpus 
> (%u)\n",
> -                    sockets, cores, threads, max_cpus);
> +            error_report("cpu topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) > "
> +                         "maxcpus (%u)",
> +                         sockets, cores, threads, max_cpus);
>              exit(1);
>          }
>  
> @@ -1246,11 +1247,11 @@ static void smp_parse(QemuOpts *opts)
>      }
>  
>      if (max_cpus > MAX_CPUMASK_BITS) {
> -        fprintf(stderr, "Unsupported number of maxcpus\n");
> +        error_report("Unsupported number of maxcpus");

Unsupported should probably start with a small 'u'

>          exit(1);
>      }
>      if (max_cpus < smp_cpus) {
> -        fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
> +        error_report("maxcpus must be equal to or greater than smp");
>          exit(1);
>      }
>  
> @@ -1260,7 +1261,7 @@ static void realtime_init(void)
>  {
>      if (enable_mlock) {
>          if (os_mlock() < 0) {
> -            fprintf(stderr, "qemu: locking memory failed\n");
> +            error_report("locking memory failed");
>              exit(1);
>          }
>      }
> @@ -1414,7 +1415,7 @@ static int usb_parse(const char *cmdline)
>      int r;
>      r = usb_device_add(cmdline);
>      if (r < 0) {
> -        fprintf(stderr, "qemu: could not add USB device '%s'\n", cmdline);
> +        error_report("could not add USB device '%s'", cmdline);
>      }
>      return r;
>  }
> @@ -1980,28 +1981,28 @@ static void select_vgahw (const char *p)
>          if (vga_available()) {
>              vga_interface_type = VGA_STD;
>          } else {
> -            fprintf(stderr, "Error: standard VGA not available\n");
> +            error_report("standard VGA not available");
>              exit(0);
>          }
>      } else if (strstart(p, "cirrus", &opts)) {
>          if (cirrus_vga_available()) {
>              vga_interface_type = VGA_CIRRUS;
>          } else {
> -            fprintf(stderr, "Error: Cirrus VGA not available\n");
> +            error_report("Cirrus VGA not available");
>              exit(0);
>          }
>      } else if (strstart(p, "vmware", &opts)) {
>          if (vmware_vga_available()) {
>              vga_interface_type = VGA_VMWARE;
>          } else {
> -            fprintf(stderr, "Error: VMWare SVGA not available\n");
> +            error_report("VMWare SVGA not available");
>              exit(0);
>          }
>      } else if (strstart(p, "virtio", &opts)) {
>          if (virtio_vga_available()) {
>              vga_interface_type = VGA_VIRTIO;
>          } else {
> -            fprintf(stderr, "Error: Virtio VGA not available\n");
> +            error_report("Virtio VGA not available");
>              exit(0);
>          }
>      } else if (strstart(p, "xenfb", &opts)) {
> @@ -2010,26 +2011,26 @@ static void select_vgahw (const char *p)
>          if (qxl_vga_available()) {
>              vga_interface_type = VGA_QXL;
>          } else {
> -            fprintf(stderr, "Error: QXL VGA not available\n");
> +            error_report("QXL VGA not available");
>              exit(0);
>          }
>      } else if (strstart(p, "tcx", &opts)) {
>          if (tcx_vga_available()) {
>              vga_interface_type = VGA_TCX;
>          } else {
> -            fprintf(stderr, "Error: TCX framebuffer not available\n");
> +            error_report("TCX framebuffer not available");
>              exit(0);
>          }
>      } else if (strstart(p, "cg3", &opts)) {
>          if (cg3_vga_available()) {
>              vga_interface_type = VGA_CG3;
>          } else {
> -            fprintf(stderr, "Error: CG3 framebuffer not available\n");
> +            error_report("CG3 framebuffer not available");
>              exit(0);
>          }
>      } else if (!strstart(p, "none", &opts)) {
>      invalid_vga:
> -        fprintf(stderr, "Unknown vga type: %s\n", p);
> +        error_report("Unknown vga type: %s", p);

lowercase u

>          exit(1);
>      }
>      while (*opts) {
> @@ -2349,7 +2350,7 @@ static int mon_init_func(void *opaque, QemuOpts *opts, 
> Error **errp)
>      } else if (strcmp(mode, "control") == 0) {
>          flags = MONITOR_USE_CONTROL;
>      } else {
> -        fprintf(stderr, "unknown monitor mode \"%s\"\n", mode);
> +        error_report("unknown monitor mode \"%s\"", mode);
>          exit(1);
>      }
>  
> @@ -2362,7 +2363,7 @@ static int mon_init_func(void *opaque, QemuOpts *opts, 
> Error **errp)
>      chardev = qemu_opt_get(opts, "chardev");
>      chr = qemu_chr_find(chardev);
>      if (chr == NULL) {
> -        fprintf(stderr, "chardev \"%s\" not found\n", chardev);
> +        error_report("chardev \"%s\" not found", chardev);
>          exit(1);
>      }
>  
> @@ -2390,7 +2391,7 @@ static void monitor_parse(const char *optarg, const 
> char *mode, bool pretty)
>          }
>          opts = qemu_chr_parse_compat(label, optarg);
>          if (!opts) {
> -            fprintf(stderr, "parse error: %s\n", optarg);
> +            error_report("parse error: %s", optarg);
>              exit(1);
>          }
>      }
> @@ -2464,14 +2465,14 @@ static int serial_parse(const char *devname)
>      if (strcmp(devname, "none") == 0)
>          return 0;
>      if (index == MAX_SERIAL_PORTS) {
> -        fprintf(stderr, "qemu: too many serial ports\n");
> +        error_report("too many serial ports");
>          exit(1);
>      }
>      snprintf(label, sizeof(label), "serial%d", index);
>      serial_hds[index] = qemu_chr_new(label, devname, NULL);
>      if (!serial_hds[index]) {
> -        fprintf(stderr, "qemu: could not connect serial device"
> -                " to character backend '%s'\n", devname);
> +        error_report("could not connect serial device"
> +                     " to character backend '%s'", devname);
>          return -1;
>      }
>      index++;
> @@ -2486,14 +2487,14 @@ static int parallel_parse(const char *devname)
>      if (strcmp(devname, "none") == 0)
>          return 0;
>      if (index == MAX_PARALLEL_PORTS) {
> -        fprintf(stderr, "qemu: too many parallel ports\n");
> +        error_report("too many parallel ports");
>          exit(1);
>      }
>      snprintf(label, sizeof(label), "parallel%d", index);
>      parallel_hds[index] = qemu_chr_new(label, devname, NULL);
>      if (!parallel_hds[index]) {
> -        fprintf(stderr, "qemu: could not connect parallel device"
> -                " to character backend '%s'\n", devname);
> +        error_report("could not connect parallel device"
> +                     " to character backend '%s'", devname);
>          return -1;
>      }
>      index++;
> @@ -2510,7 +2511,7 @@ static int virtcon_parse(const char *devname)
>      if (strcmp(devname, "none") == 0)
>          return 0;
>      if (index == MAX_VIRTIO_CONSOLES) {
> -        fprintf(stderr, "qemu: too many virtio consoles\n");
> +        error_report("too many virtio consoles");
>          exit(1);
>      }
>  
> @@ -2527,8 +2528,8 @@ static int virtcon_parse(const char *devname)
>      snprintf(label, sizeof(label), "virtcon%d", index);
>      virtcon_hds[index] = qemu_chr_new(label, devname, NULL);
>      if (!virtcon_hds[index]) {
> -        fprintf(stderr, "qemu: could not connect virtio console"
> -                " to character backend '%s'\n", devname);
> +        error_report("could not connect virtio console"
> +                     " to character backend '%s'", devname);
>          return -1;
>      }
>      qemu_opt_set(dev_opts, "chardev", label, &error_abort);
> @@ -2548,7 +2549,7 @@ static int sclp_parse(const char *devname)
>          return 0;
>      }
>      if (index == MAX_SCLP_CONSOLES) {
> -        fprintf(stderr, "qemu: too many sclp consoles\n");
> +        error_report("too many sclp consoles");
>          exit(1);
>      }
>  
> @@ -2560,8 +2561,8 @@ static int sclp_parse(const char *devname)
>      snprintf(label, sizeof(label), "sclpcon%d", index);
>      sclp_hds[index] = qemu_chr_new(label, devname, NULL);
>      if (!sclp_hds[index]) {
> -        fprintf(stderr, "qemu: could not connect sclp console"
> -                " to character backend '%s'\n", devname);
> +        error_report("could not connect sclp console"
> +                     " to character backend '%s'", devname);
>          return -1;
>      }
>      qemu_opt_set(dev_opts, "chardev", label, &error_abort);
> @@ -2579,7 +2580,7 @@ static int debugcon_parse(const char *devname)
>      }
>      opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1, NULL);
>      if (!opts) {
> -        fprintf(stderr, "qemu: already have a debugcon device\n");
> +        error_report("already have a debugcon device");
>          exit(1);
>      }
>      qemu_opt_set(opts, "driver", "isa-debugcon", &error_abort);
> @@ -3005,8 +3006,7 @@ int main(int argc, char **argv, char **envp)
>      runstate_init();
>  
>      if (qcrypto_init(&err) < 0) {
> -        fprintf(stderr, "Cannot initialize crypto: %s\n",
> -                error_get_pretty(err));
> +        error_report("Cannot initialize crypto: %s", error_get_pretty(err));

lowercase 'c'

>          exit(1);
>      }
>      rtc_clock = QEMU_CLOCK_HOST;
> @@ -3164,7 +3164,7 @@ int main(int argc, char **argv, char **envp)
>                          }
>                      } else if (*p != '\0') {
>                      chs_fail:
> -                        fprintf(stderr, "qemu: invalid physical CHS 
> format\n");
> +                        error_report("invalid physical CHS format");
>                          exit(1);
>                      }
>                      if (hda_opts != NULL) {
> @@ -3207,7 +3207,7 @@ int main(int argc, char **argv, char **envp)
>  #ifdef CONFIG_CURSES
>                  display_type = DT_CURSES;
>  #else
> -                fprintf(stderr, "Curses support is disabled\n");
> +                error_report("Curses support is disabled");
>                  exit(1);
>  #endif
>                  break;
> @@ -3218,8 +3218,7 @@ int main(int argc, char **argv, char **envp)
>                  graphic_rotate = strtol(optarg, (char **) &optarg, 10);
>                  if (graphic_rotate != 0 && graphic_rotate != 90 &&
>                      graphic_rotate != 180 && graphic_rotate != 270) {
> -                    fprintf(stderr,
> -                        "qemu: only 90, 180, 270 deg rotation is 
> available\n");
> +                    error_report("only 90, 180, 270 deg rotation is 
> available");
>                      exit(1);
>                  }
>                  break;
> @@ -3370,7 +3369,7 @@ int main(int argc, char **argv, char **envp)
>                      w = strtol(p, (char **)&p, 10);
>                      if (w <= 0) {
>                      graphic_error:
> -                        fprintf(stderr, "qemu: invalid resolution or 
> depth\n");
> +                        error_report("invalid resolution or depth");
>                          exit(1);
>                      }
>                      if (*p != 'x')
> @@ -3436,7 +3435,7 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_fsdev:
>                  olist = qemu_find_opts("fsdev");
>                  if (!olist) {
> -                    fprintf(stderr, "fsdev is not supported by this qemu 
> build.\n");
> +                    error_report("fsdev is not supported by this qemu 
> build.");
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(olist, optarg, true);
> @@ -3451,7 +3450,7 @@ int main(int argc, char **argv, char **envp)
>  
>                  olist = qemu_find_opts("virtfs");
>                  if (!olist) {
> -                    fprintf(stderr, "virtfs is not supported by this qemu 
> build.\n");
> +                    error_report("virtfs is not supported by this qemu 
> build.");
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(olist, optarg, true);
> @@ -3461,15 +3460,15 @@ int main(int argc, char **argv, char **envp)
>  
>                  if (qemu_opt_get(opts, "fsdriver") == NULL ||
>                      qemu_opt_get(opts, "mount_tag") == NULL) {
> -                    fprintf(stderr, "Usage: -virtfs 
> fsdriver,mount_tag=tag.\n");
> +                    error_report("Usage: -virtfs fsdriver,mount_tag=tag.");

lowercase 'u' (remove period as suggested by Markus)

>                      exit(1);
>                  }
>                  fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
>                                           qemu_opt_get(opts, "mount_tag"),
>                                           1, NULL);
>                  if (!fsdev) {
> -                    fprintf(stderr, "duplicate fsdev id: %s\n",
> -                            qemu_opt_get(opts, "mount_tag"));
> +                    error_report("duplicate fsdev id: %s",
> +                                 qemu_opt_get(opts, "mount_tag"));
>                      exit(1);
>                  }
>  
> @@ -3478,8 +3477,7 @@ int main(int argc, char **argv, char **envp)
>  #ifdef CONFIG_SYNC_FILE_RANGE
>                      qemu_opt_set(fsdev, "writeout", writeout, &error_abort);
>  #else
> -                    fprintf(stderr, "writeout=immediate not supported on "
> -                            "this platform\n");
> +                    error_report("writeout=immediate not supported on this 
> platform");
>                      exit(1);
>  #endif
>                  }
> @@ -3518,7 +3516,7 @@ int main(int argc, char **argv, char **envp)
>                  fsdev = qemu_opts_create(qemu_find_opts("fsdev"), "v_synth",
>                                           1, NULL);
>                  if (!fsdev) {
> -                    fprintf(stderr, "duplicate option: %s\n", 
> "virtfs_synth");
> +                    error_report("duplicate option: %s", "virtfs_synth");
>                      exit(1);
>                  }
>                  qemu_opt_set(fsdev, "fsdriver", "synth", &error_abort);
> @@ -3539,15 +3537,14 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_watchdog:
>                  if (watchdog) {
> -                    fprintf(stderr,
> -                            "qemu: only one watchdog option may be given\n");
> +                    error_report("only one watchdog option may be given");
>                      return 1;
>                  }
>                  watchdog = optarg;
>                  break;
>              case QEMU_OPTION_watchdog_action:
>                  if (select_watchdog_action(optarg) == -1) {
> -                    fprintf(stderr, "Unknown -watchdog-action parameter\n");
> +                    error_report("Unknown -watchdog-action parameter");

lowercase 'u'

>                      exit(1);
>                  }
>                  break;
> @@ -3591,7 +3588,7 @@ int main(int argc, char **argv, char **envp)
>                  display_type = DT_SDL;
>                  break;
>  #else
> -                fprintf(stderr, "SDL support is disabled\n");
> +                error_report("SDL support is disabled");
>                  exit(1);
>  #endif
>              case QEMU_OPTION_pidfile:
> @@ -3653,8 +3650,8 @@ int main(int argc, char **argv, char **envp)
>                  qemu_opts_parse_noisily(olist, "accel=tcg", false);
>                  break;
>              case QEMU_OPTION_no_kvm_pit: {
> -                fprintf(stderr, "Warning: KVM PIT can no longer be disabled "
> -                                "separately.\n");
> +                error_report("Warning: KVM PIT can no longer be disabled "
> +                             "separately.");

Could change this from a sentence into a phrase. Also, we need a consistent
'warning' prefix. Should we make a error_report_warn variant?

>                  break;
>              }
>              case QEMU_OPTION_no_kvm_pit_reinjection: {
> @@ -3667,8 +3664,8 @@ int main(int argc, char **argv, char **envp)
>                      { /* end of list */ }
>                  };
>  
> -                fprintf(stderr, "Warning: option deprecated, use "
> -                        "lost_tick_policy property of kvm-pit instead.\n");
> +                error_report("Warning: option deprecated, use "
> +                             "lost_tick_policy property of kvm-pit 
> instead.");

same as last comment

>                  qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
>                  break;
>              }
> @@ -3703,7 +3700,7 @@ int main(int argc, char **argv, char **envp)
>                      exit(1);
>                  }
>  #else
> -                fprintf(stderr, "VNC support is disabled\n");
> +                error_report("VNC support is disabled");
>                  exit(1);
>  #endif
>                  break;
> @@ -3716,7 +3713,7 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_balloon:
>                  if (balloon_parse(optarg) < 0) {
> -                    fprintf(stderr, "Unknown -balloon argument %s\n", 
> optarg);
> +                    error_report("Unknown -balloon argument %s", optarg);

lowercase 'u'

>                      exit(1);
>                  }
>                  break;
> @@ -3731,15 +3728,15 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_uuid:
>                  if(qemu_uuid_parse(optarg, qemu_uuid) < 0) {
> -                    fprintf(stderr, "Fail to parse UUID string."
> -                            " Wrong format.\n");
> +                    error_report("Fail to parse UUID string."

should use past tense "Failed"

> +                                 " Wrong format.");
>                      exit(1);
>                  }
>                  qemu_uuid_set = true;
>                  break;
>              case QEMU_OPTION_option_rom:
>                  if (nb_option_roms >= MAX_OPTION_ROMS) {
> -                    fprintf(stderr, "Too many option ROMs\n");
> +                    error_report("Too many option ROMs");

lowercase 't'
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(qemu_find_opts("option-rom"),
> @@ -3751,7 +3748,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) {
> -                    fprintf(stderr, "Option ROM file is not specified\n");
> +                    error_report("Option ROM file is not specified");
lowercase 'o'
>                      exit(1);
>                  }
>                  nb_option_roms++;
> @@ -3776,9 +3773,8 @@ int main(int argc, char **argv, char **envp)
>                          } else  if (strcmp("auto", target) == 0) {
>                              semihosting.target = SEMIHOSTING_TARGET_AUTO;
>                          } else {
> -                            fprintf(stderr, "Unsupported semihosting-config"
> -                                    " %s\n",
> -                                optarg);
> +                            error_report("Unsupported semihosting-config %s",
lowercase 'u'
> +                                         optarg);
>                              exit(1);
>                          }
>                      } else {
> @@ -3788,14 +3784,14 @@ int main(int argc, char **argv, char **envp)
>                      qemu_opt_foreach(opts, add_semihosting_arg,
>                                       &semihosting, NULL);
>                  } else {
> -                    fprintf(stderr, "Unsupported semihosting-config %s\n",
> -                            optarg);
> +                    error_report("Unsupported semihosting-config %s",
lowercase 'u'
> +                                 optarg);
>                      exit(1);
>                  }
>                  break;
>              case QEMU_OPTION_tdf:
> -                fprintf(stderr, "Warning: user space PIT time drift fix "
> -                                "is no longer supported.\n");
> +                error_report("Warning: user space PIT time drift fix "
> +                             "is no longer supported.");

no period and "error_report_warn"

>                  break;
>              case QEMU_OPTION_name:
>                  opts = qemu_opts_parse_noisily(qemu_find_opts("name"),
> @@ -3806,7 +3802,7 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_prom_env:
>                  if (nb_prom_envs >= MAX_PROM_ENVS) {
> -                    fprintf(stderr, "Too many prom variables\n");
> +                    error_report("Too many prom variables");

lowercase 't'

>                      exit(1);
>                  }
>                  prom_envs[nb_prom_envs] = optarg;
> @@ -3889,8 +3885,8 @@ int main(int argc, char **argv, char **envp)
>                  {
>                      int ret = qemu_read_config_file(optarg);
>                      if (ret < 0) {
> -                        fprintf(stderr, "read config %s: %s\n", optarg,
> -                            strerror(-ret));
> +                        error_report("read config %s: %s", optarg,
> +                                     strerror(-ret));
>                          exit(1);
>                      }
>                      break;
> @@ -3898,7 +3894,7 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_spice:
>                  olist = qemu_find_opts("spice");
>                  if (!olist) {
> -                    fprintf(stderr, "spice is not supported by this qemu 
> build.\n");
> +                    error_report("spice is not supported by this qemu 
> build.");
no period
>                      exit(1);
>                  }
>                  opts = qemu_opts_parse_noisily(olist, optarg, false);
> @@ -3915,7 +3911,7 @@ int main(int argc, char **argv, char **envp)
>                      } else {
>                          fp = fopen(optarg, "w");
>                          if (fp == NULL) {
> -                            fprintf(stderr, "open %s: %s\n", optarg, 
> strerror(errno));
> +                            error_report("open %s: %s", optarg, 
> strerror(errno));
>                              exit(1);
>                          }
>                      }
> @@ -3976,13 +3972,13 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              case QEMU_OPTION_dump_vmstate:
>                  if (vmstate_dump_file) {
> -                    fprintf(stderr, "qemu: only one '-dump-vmstate' "
> -                            "option may be given\n");
> +                    error_report("only one '-dump-vmstate' "
> +                                 "option may be given");
>                      exit(1);
>                  }
>                  vmstate_dump_file = fopen(optarg, "w");
>                  if (vmstate_dump_file == NULL) {
> -                    fprintf(stderr, "open %s: %s\n", optarg, 
> strerror(errno));
> +                    error_report("open %s: %s", optarg, strerror(errno));
>                      exit(1);
>                  }
>                  break;
> @@ -3999,8 +3995,8 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if (machine_class == NULL) {
> -        fprintf(stderr, "No machine specified, and there is no default.\n"
> -                "Use -machine help to list supported machines!\n");
> +        error_report("No machine specified, and there is no default.\n"
> +                "Use -machine help to list supported machines!");
>          exit(1);
>      }
>  
> @@ -4101,9 +4097,9 @@ int main(int argc, char **argv, char **envp)
>  
>      machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP 
> */
>      if (max_cpus > machine_class->max_cpus) {
> -        fprintf(stderr, "Number of SMP CPUs requested (%d) exceeds max CPUs "
> -                "supported by machine '%s' (%d)\n", max_cpus,
> -                machine_class->name, machine_class->max_cpus);
> +        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> +                     "supported by machine '%s' (%d)", max_cpus,
> +                     machine_class->name, machine_class->max_cpus);
>          exit(1);
>      }
>  
> @@ -4164,12 +4160,12 @@ int main(int argc, char **argv, char **envp)
>          if (display_type == DT_NOGRAPHIC
>              && (default_parallel || default_serial
>                  || default_monitor || default_virtcon)) {
> -            fprintf(stderr, "-nographic can not be used with -daemonize\n");
> +            error_report("-nographic can not be used with -daemonize");
>              exit(1);
>          }
>  #ifdef CONFIG_CURSES
>          if (display_type == DT_CURSES) {
> -            fprintf(stderr, "curses display can not be used with 
> -daemonize\n");
> +            error_report("curses display can not be used with -daemonize");

Capital 'C' for the proper noun 'Curses' ?

>              exit(1);
>          }
>  #endif
> @@ -4228,12 +4224,12 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
> -        fprintf(stderr, "-no-frame, -alt-grab and -ctrl-grab are only valid "
> -                        "for SDL, ignoring option\n");
> +        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
> +                     "for SDL, ignoring option");
>      }
>      if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
> -        fprintf(stderr, "-no-quit is only valid for GTK and SDL, "
> -                        "ignoring option\n");
> +        error_report("-no-quit is only valid for GTK and SDL, "
> +                     "ignoring option");
>      }
>  
>  #if defined(CONFIG_GTK)
> @@ -4248,9 +4244,9 @@ int main(int argc, char **argv, char **envp)
>  #endif
>      if (request_opengl == 1 && display_opengl == 0) {
>  #if defined(CONFIG_OPENGL)
> -        fprintf(stderr, "OpenGL is not supported by the display.\n");
> +        error_report("OpenGL is not supported by the display.");
>  #else
> -        fprintf(stderr, "QEMU was built without opengl support.\n");
> +        error_report("QEMU was built without opengl support.");
>  #endif
>          exit(1);
>      }
> @@ -4276,7 +4272,7 @@ int main(int argc, char **argv, char **envp)
>  #endif
>  
>      if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> -        fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
> +        error_report("Could not acquire pid file: %s", strerror(errno));
lowercase 'c'
>          exit(1);
>      }
>  
> @@ -4347,17 +4343,17 @@ int main(int argc, char **argv, char **envp)
>      linux_boot = (kernel_filename != NULL);
>  
>      if (!linux_boot && *kernel_cmdline != '\0') {
> -        fprintf(stderr, "-append only allowed with -kernel option\n");
> +        error_report("-append only allowed with -kernel option");
>          exit(1);
>      }
>  
>      if (!linux_boot && initrd_filename != NULL) {
> -        fprintf(stderr, "-initrd only allowed with -kernel option\n");
> +        error_report("-initrd only allowed with -kernel option");
>          exit(1);
>      }
>  
>      if (!linux_boot && qemu_opt_get(machine_opts, "dtb")) {
> -        fprintf(stderr, "-dtb only allowed with -kernel option\n");
> +        error_report("-dtb only allowed with -kernel option");
>          exit(1);
>      }
>  
> @@ -4376,7 +4372,7 @@ int main(int argc, char **argv, char **envp)
>      cpu_ticks_init();
>      if (icount_opts) {
>          if (kvm_enabled() || xen_enabled()) {
> -            fprintf(stderr, "-icount is not allowed with kvm or xen\n");
> +            error_report("-icount is not allowed with kvm or xen");
>              exit(1);
>          }
>          configure_icount(icount_opts, &error_abort);
> @@ -4409,7 +4405,7 @@ int main(int argc, char **argv, char **envp)
>      if (!xen_enabled()) {
>          /* On 32-bit hosts, QEMU is limited by virtual address space */
>          if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
> -            fprintf(stderr, "qemu: at most 2047 MB RAM can be simulated\n");
> +            error_report("at most 2047 MB RAM can be simulated");
>              exit(1);
>          }
>      }
> @@ -4596,7 +4592,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_run_machine_init_done_notifiers();
>  
>      if (rom_check_and_register_reset() != 0) {
> -        fprintf(stderr, "rom check and register reset failed\n");
> +        error_report("rom check and register reset failed");
>          exit(1);
>      }
>  
> -- 
> 2.1.0
> 
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]