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

Re: [libvirt] [PATCH] conf: fix missing spaces in message



On 09/12/2012 12:57 PM, Eric Blake wrote:
> I got an off-list report about a bad diagnostic:
> Target network card mac 52:54:00:49:07:ccdoes not match source 52:54:00:49:07:b8
>
> True to form, I've added a syntax check rule to prevent it
> from recurring, and found several other offenders.
>
> * cfg.mk (sc_require_whitespace_in_translation): New rule.
> * src/conf/domain_conf.c (virDomainNetDefCheckABIStability): Add
> space.
> * src/esx/esx_util.c (esxUtil_ParseUri): Likewise.
> * src/qemu/qemu_command.c (qemuCollectPCIAddress): Likewise.
> * src/qemu/qemu_driver.c (qemuDomainSetMetadata)
> (qemuDomainGetMetadata): Likewise.
> * src/qemu/qemu_hotplug.c (qemuDomainChangeNetBridge): Likewise.
> * src/rpc/virnettlscontext.c
> (virNetTLSContextCheckCertDNWhitelist): Likewise.
> * src/vmware/vmware_driver.c (vmwareDomainResume): Likewise.
> * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc, vboxAttachDrives):
> Avoid false negatives.
> * tools/virsh-domain.c (info_save_image_dumpxml): Reword.
> Based on a report by Luwen Su.
> ---
>
> Too big to use the trivial rule, so I'll wait for a review.
>
>  cfg.mk                     |  9 +++++++++
>  src/conf/domain_conf.c     |  5 +++--
>  src/esx/esx_util.c         |  7 ++++---
>  src/qemu/qemu_command.c    |  5 +++--
>  src/qemu/qemu_driver.c     |  6 +++---
>  src/qemu/qemu_hotplug.c    |  2 +-
>  src/rpc/virnettlscontext.c |  6 +++---
>  src/vbox/vbox_tmpl.c       | 14 ++++++++------
>  src/vmware/vmware_driver.c |  4 ++--
>  tools/virsh-domain.c       |  4 ++--
>  10 files changed, 38 insertions(+), 24 deletions(-)
>
> diff --git a/cfg.mk b/cfg.mk
> index bca363c..0dd58df 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -597,6 +597,15 @@ sc_prohibit_useless_translation:
>  	halt='no translations in tests or examples'			\
>  	  $(_sc_search_regexp)
>
> +# When splitting a diagnostic across lines, ensure that there is a space
> +# or \n on one side of the split.
> +sc_require_whitespace_in_translation:
> +	@grep -n -A1 '"$$' $$($(VC_LIST_EXCEPT))   			\
> +	   | sed -ne ':l; /"$$/ {N;b l;}; s/"\n[^"]*"/""/g; s/\\n/ /g'	\
> +		-e '/_(.*[^\ ]""[^\ ]/p' | grep . &&			\
> +	  { echo '$(ME): missing whitespace at line split' 1>&2;	\
> +	    exit 1; } || :
> +
>  # Enforce recommended preprocessor indentation style.
>  sc_preprocessor_indentation:
>  	@if cppi --version >/dev/null 2>&1; then			\
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0e71b06..292cc9a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9846,7 +9846,8 @@ static bool virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
>              src->addr.pci.slot != dst->addr.pci.slot ||
>              src->addr.pci.function != dst->addr.pci.function) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                           _("Target device PCI address %04x:%02x:%02x.%02x does not match source %04x:%02x:%02x.%02x"),
> +                           _("Target device PCI address %04x:%02x:%02x.%02x "
> +                             "does not match source %04x:%02x:%02x.%02x"),
>                             dst->addr.pci.domain, dst->addr.pci.bus,
>                             dst->addr.pci.slot, dst->addr.pci.function,
>                             src->addr.pci.domain, src->addr.pci.bus,
> @@ -10044,7 +10045,7 @@ static bool virDomainNetDefCheckABIStability(virDomainNetDefPtr src,
>      if (virMacAddrCmp(&src->mac, &dst->mac) != 0) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("Target network card mac %02x:%02x:%02x:%02x:%02x:%02x"
> -                         "does not match source %02x:%02x:%02x:%02x:%02x:%02x"),
> +                         " does not match source %02x:%02x:%02x:%02x:%02x:%02x"),

I curious if there's any reason why you added a space at the end of the
first line in all other cases, but at the beginning of the 2nd line in
this case (I suppose if nothing else, it proves that your syntax-check
rule works correctly in both cases :-)

At any rate, ACK.


>                         dst->mac.addr[0], dst->mac.addr[1], dst->mac.addr[2],
>                         dst->mac.addr[3], dst->mac.addr[4], dst->mac.addr[5],
>                         src->mac.addr[0], src->mac.addr[1], src->mac.addr[2],
> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
> index 9288218..9d84a6a 100644
> --- a/src/esx/esx_util.c
> +++ b/src/esx/esx_util.c
> @@ -2,7 +2,7 @@
>  /*
>   * esx_util.c: utility functions for the VMware ESX driver
>   *
> - * Copyright (C) 2010-2011 Red Hat, Inc.
> + * Copyright (C) 2010-2012 Red Hat, Inc.
>   * Copyright (C) 2009-2011 Matthias Bolte <matthias bolte googlemail com>
>   * Copyright (C) 2009 Maximilian Wilhelm <max rfc2324 org>
>   *
> @@ -159,8 +159,9 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri)
>                      (*parsedUri)->proxy_port < 1 ||
>                      (*parsedUri)->proxy_port > 65535) {
>                      virReportError(VIR_ERR_INVALID_ARG,
> -                                   _("Query parameter 'proxy' has unexpected port"
> -                                     "value '%s' (should be [1..65535])"), tmp);
> +                                   _("Query parameter 'proxy' has unexpected "
> +                                     "port value '%s' (should be [1..65535])"),
> +                                   tmp);
>                      goto cleanup;
>                  }
>              }
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a83d6de..cd4ee93 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1044,8 +1044,9 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
>
>              if (virHashLookup(addrs->used, addr)) {
>                  virReportError(VIR_ERR_XML_ERROR,
> -                               _("Attempted double use of PCI Address '%s'"
> -                                 "(need \"multifunction='off'\" for device on function 0)"),
> +                               _("Attempted double use of PCI Address '%s' "
> +                                 "(need \"multifunction='off'\" for device "
> +                                 "on function 0)"),
>                                 addr);
>                  goto cleanup;
>              }
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 8e8e00c..a410521 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13271,7 +13271,7 @@ qemuDomainSetMetadata(virDomainPtr dom,
>              break;
>          case VIR_DOMAIN_METADATA_ELEMENT:
>              virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                           _("QEmu driver does not support modifying"
> +                           _("QEmu driver does not support modifying "
>                               "<metadata> element"));
>              goto cleanup;
>              break;
> @@ -13299,7 +13299,7 @@ qemuDomainSetMetadata(virDomainPtr dom,
>              break;
>          case VIR_DOMAIN_METADATA_ELEMENT:
>              virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                           _("QEMU driver does not support"
> +                           _("QEMU driver does not support "
>                               "<metadata> element"));
>              goto cleanup;
>           default:
> @@ -13367,7 +13367,7 @@ qemuDomainGetMetadata(virDomainPtr dom,
>          break;
>      case VIR_DOMAIN_METADATA_ELEMENT:
>          virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -                       _("QEMU driver does not support"
> +                       _("QEMU driver does not support "
>                           "<metadata> element"));
>          goto cleanup;
>          break;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index a8a904c..d4d08ac 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1278,7 +1278,7 @@ int qemuDomainChangeNetBridge(virDomainObjPtr vm,
>          virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
>          if (ret < 0) {
>              virReportError(VIR_ERR_OPERATION_FAILED,
> -                           _("unable to recover former state by adding port"
> +                           _("unable to recover former state by adding port "
>                               "to bridge %s"), oldbridge);
>          }
>          return -1;
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index 9fe6eb1..dee4334 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c
> @@ -1,7 +1,7 @@
>  /*
>   * virnettlscontext.c: TLS encryption/x509 handling
>   *
> - * Copyright (C) 2010-2011 Red Hat, Inc.
> + * Copyright (C) 2010-2012 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -393,8 +393,8 @@ virNetTLSContextCheckCertDNWhitelist(const char *dname,
>      virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
>                     _("Client's Distinguished Name is not on the list "
>                       "of allowed clients (tls_allowed_dn_list).  Use "
> -                     "'certtool -i --infile clientcert.pem' to view the"
> -                     "Distinguished Name field in the client certificate,"
> +                     "'certtool -i --infile clientcert.pem' to view the "
> +                     "Distinguished Name field in the client certificate, "
>                       "or run this daemon with --verbose option."));
>      return 0;
>  }
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 48f371f..4f2d025 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -3752,8 +3752,9 @@ static int vboxDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) {
>                      ret = vboxStartMachine(dom, i, machine, &iid);
>                  } else {
>                      virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> -                                   _("machine is not in poweroff|saved|"
> -                                     "aborted state, so couldn't start it"));
> +                                   _("machine is not in "
> +                                     "poweroff|saved|aborted state, so "
> +                                     "couldn't start it"));
>                      ret = -1;
>                  }
>              }
> @@ -4280,8 +4281,9 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
>                                        &devicePort,
>                                        &deviceSlot)) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("can't get the port/slot number of harddisk/"
> -                                 "dvd/floppy to be attached: %s, rc=%08x"),
> +                               _("can't get the port/slot number of "
> +                                 "harddisk/dvd/floppy to be attached: "
> +                                 "%s, rc=%08x"),
>                                 def->disks[i]->src, (unsigned)rc);
>                  VBOX_RELEASE(medium);
>                  VBOX_UTF16_FREE(mediumUUID);
> @@ -4303,8 +4305,8 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
>
>              if (NS_FAILED(rc)) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
> -                               _("could not attach the file as harddisk/"
> -                                 "dvd/floppy: %s, rc=%08x"),
> +                               _("could not attach the file as "
> +                                 "harddisk/dvd/floppy: %s, rc=%08x"),
>                                 def->disks[i]->src, (unsigned)rc);
>              } else {
>                  DEBUGIID("Attached HDD/DVD/Floppy with UUID", mediumUUID);
> diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
> index 1607018..557b917 100644
> --- a/src/vmware/vmware_driver.c
> +++ b/src/vmware/vmware_driver.c
> @@ -1,6 +1,6 @@
>  /*---------------------------------------------------------------------------*/
>  /*
> - * Copyright (C) 2011 Red Hat, Inc.
> + * Copyright (C) 2011-2012 Red Hat, Inc.
>   * Copyright 2010, diateam (www.diateam.net)
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -484,7 +484,7 @@ vmwareDomainResume(virDomainPtr dom)
>
>      if (driver->type == TYPE_PLAYER) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("vmplayer does not support libvirt suspend/resume"
> +                       _("vmplayer does not support libvirt suspend/resume "
>                           "(vmware pause/unpause) operation "));
>          return ret;
>      }
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 409eb24..c6695b3 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2945,8 +2945,8 @@ cleanup:
>   */
>  static const vshCmdInfo info_save_image_dumpxml[] = {
>      {"help", N_("saved state domain information in XML")},
> -    {"desc", N_("Output the domain information for a saved state file,\n"
> -                "as an XML dump to stdout.")},
> +    {"desc",
> +     N_("Dump XML of domain information for a saved state file to stdout.")},
>      {NULL, NULL}
>  };
>


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