[libvirt] [PATCH] avoid one more ctype vs. sign-extension problem

[Daniel Veillard]

On Fri, May 09, 2008 at 12:13:50PM +0200, Jim Meyering wrote:
> Daniel Veillard <veillard at redhat.com> 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 at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/