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(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/