[libvirt] [PATCH] maint: avoid locale-sensitivity in string case comparisons

[Daniel Veillard]

On Wed, Mar 30, 2011 at 08:29:53PM -0600, Eric Blake wrote:
> strcase{cmp/str} have the drawback of being sensitive to the global
> locale; this is unacceptable in a library setting.  Prefer a
> hard-coded C locale alternative for all but virsh, which is user
> facing and where the global locale isn't changing externally.
> 
> * .gnulib: Update to latest, for c-strcasestr change.
> * bootstrap.conf (gnulib_modules): Drop strcasestr, add c-strcase
> and c-strcasestr.
> * cfg.mk (sc_avoid_strcase): New rule.
> (exclude_file_name_regexp--sc_avoid_strcase): New exception.
> * src/internal.h (STRCASEEQ, STRCASENEQ, STRCASEEQLEN)
> (STRCASENEQLEN): Adjust offenders.
> * src/qemu/qemu_monitor_text.c (qemuMonitorTextEjectMedia):
> Likewise.
> * tools/virsh.c (namesorter): Document exception.
> ---
> 
> Inspired by today's earlier patch that started using strcasester.
> This also goes along with our policy of no ctype, just c-ctype.
> 
>  .gnulib                      |    2 +-
>  bootstrap.conf               |    3 ++-
>  cfg.mk                       |    7 +++++++
>  src/internal.h               |   10 ++++++----
>  src/qemu/qemu_monitor_text.c |    3 ++-
>  tools/virsh.c                |    1 +
>  6 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/.gnulib b/.gnulib
> index 422ab2e..790645d 160000
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 422ab2e0d70ed348e2fd0a82558be38e5859011a
> +Subproject commit 790645d837f8084991421107fba639b110d58335
> diff --git a/bootstrap.conf b/bootstrap.conf
> index 6e10828..733c354 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -22,6 +22,8 @@ gnulib_modules='
>  areadlink
>  base64
>  c-ctype
> +c-strcase
> +c-strcasestr
>  canonicalize-lgpl
>  chown
>  close
> @@ -63,7 +65,6 @@ sigpipe
>  snprintf
>  socket
>  stpcpy
> -strcasestr
>  strchrnul
>  strndup
>  strerror
> diff --git a/cfg.mk b/cfg.mk
> index ac419f7..f802cee 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -349,6 +349,11 @@ sc_avoid_ctype_macros:
>  	halt="don't use ctype macros (use c-ctype.h)"			\
>  	  $(_sc_search_regexp)
> 
> +sc_avoid_strcase:
> +	@prohibit='\bstrn?case(cmp|str) *\('				\
> +	halt="don't use raw strcase functions (use c-strcase instead)"	\
> +	  $(_sc_search_regexp)
> +
>  sc_prohibit_virBufferAdd_with_string_literal:
>  	@prohibit='\<virBufferAdd *\([^,]+, *"[^"]'			\
>  	halt='use virBufferAddLit, not virBufferAdd, with a string literal' \
> @@ -547,6 +552,8 @@ _makefile_at_at_check_exceptions = ' && !/(SCHEMA|SYSCONF)DIR/'
>  syntax-check: $(top_srcdir)/HACKING
> 
>  # List all syntax-check exemptions:
> +exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.c$$
> +
>  _src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal
>  exclude_file_name_regexp--sc_avoid_write = \
>    ^(src/($(_src1))|daemon/libvirtd|tools/console)\.c$$
> diff --git a/src/internal.h b/src/internal.h
> index be97801..2afbd8d 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -44,6 +44,8 @@
> 
>  # include "libvirt_internal.h"
> 
> +# include "c-strcase.h"
> +
>  /* On architectures which lack these limits, define them (ie. Cygwin).
>   * Note that the libvirt code should be robust enough to handle the
>   * case where actual value is longer than these limits (eg. by setting
> @@ -64,13 +66,13 @@
> 
>  /* String equality tests, suggested by Jim Meyering. */
>  # define STREQ(a,b) (strcmp(a,b) == 0)
> -# define STRCASEEQ(a,b) (strcasecmp(a,b) == 0)
> +# define STRCASEEQ(a,b) (c_strcasecmp(a,b) == 0)
>  # define STRNEQ(a,b) (strcmp(a,b) != 0)
> -# define STRCASENEQ(a,b) (strcasecmp(a,b) != 0)
> +# define STRCASENEQ(a,b) (c_strcasecmp(a,b) != 0)
>  # define STREQLEN(a,b,n) (strncmp(a,b,n) == 0)
> -# define STRCASEEQLEN(a,b,n) (strncasecmp(a,b,n) == 0)
> +# define STRCASEEQLEN(a,b,n) (c_strncasecmp(a,b,n) == 0)
>  # define STRNEQLEN(a,b,n) (strncmp(a,b,n) != 0)
> -# define STRCASENEQLEN(a,b,n) (strncasecmp(a,b,n) != 0)
> +# define STRCASENEQLEN(a,b,n) (c_strncasecmp(a,b,n) != 0)
>  # define STRPREFIX(a,b) (strncmp(a,b,strlen(b)) == 0)
>  # define STRSKIP(a,b) (STRPREFIX(a,b) ? (a) + strlen(b) : NULL)
> 
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index e0e3292..168c60f 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -33,6 +33,7 @@
>  #include "qemu_monitor_text.h"
>  #include "qemu_command.h"
>  #include "c-ctype.h"
> +#include "c-strcasestr.h"
>  #include "memory.h"
>  #include "logging.h"
>  #include "driver.h"
> @@ -934,7 +935,7 @@ int qemuMonitorTextEjectMedia(qemuMonitorPtr mon,
>      /* If the command failed qemu prints:
>       * device not found, device is locked ...
>       * No message is printed on success it seems */
> -    if (strcasestr(reply, "device ")) {
> +    if (c_strcasestr(reply, "device ")) {
>          qemuReportError(VIR_ERR_OPERATION_FAILED,
>                          _("could not eject media on %s: %s"), devname, reply);
>          goto cleanup;
> diff --git a/tools/virsh.c b/tools/virsh.c
> index faeaf47..3c759b9 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -432,6 +432,7 @@ static int namesorter(const void *a, const void *b) {
>    const char **sa = (const char**)a;
>    const char **sb = (const char**)b;
> 
> +  /* User visible sort, so we want locale-specific case comparison.  */
>    return strcasecmp(*sa, *sb);
>  }
> 

  ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/