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

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

 



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);
 }

-- 
1.7.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]