On Fri, May 09, 2008 at 12:13:50PM +0200, Jim Meyering wrote: > Daniel Veillard <veillard@xxxxxxxxxx> wrote: > > On Thu, May 08, 2008 at 04:44:38PM +0200, Jim Meyering wrote: > >> This change demonstrates that the new syntax-check rule's > >> regexp can be improved. It missed the unsafe tolower use, > >> since there was already a to_uchar use on that line. > > > > Since this shows up again, let me argue a bit more why I dislike > > using is* and to* in libvirt. > > I agree completely, especially since I've just fixed three > similar bugs in coreutils: Ah, good, it's not just the Frenchman being overly stubborn then :-) > http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/13512 > http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=209850fd7 > > I'm in the process of removing all of them from libvirt, since there > aren't many. Actually, I've just realized that the 'right' way to do > this is to change each use of isspace to c_isspace, isdigit to c_isdigit, > etc., relying on the gnulib c-ctype module: > > http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/c-ctype.h That looks fine, yes. > Looking into that, I see more room for improvement, i.e., where things > like isdigit and isalnum have been open-coded. With these c_is* functions, > we can have portability, readability, *and* efficiency: > > For example, compare these: > > while ((*cur >= '0') && (*cur <= '9')) { > > while (c_isdigit(*cur)) { > > ================================ > and these: > > char_ok = > (model[i] >= '0' && model[i] <= '9') || > (model[i] >= 'a' && model[i] <= 'z') || > (model[i] >= 'A' && model[i] <= 'Z') || model[i] == '_'; > > char_ok = c_isalnum (model[i]) || model[i] == '_'; > > So I'm making changes like these, too. great, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list