Re: wget 1.6 porting problem with snprintf and isdigit on Solaris 2.5.1
Quoting Paul Eggert ([EMAIL PROTECTED]): When building wget 1.6 on Solaris 2.5.1 with GCC 2.95.3, I ran into the following porting problem. snprintf.c: In function `dopr': snprintf.c:230: warning: subscript has type `char' snprintf.c:254: warning: subscript has type `char' This is warning that isdigit doesn't work on negative characters (which are possible on hosts where characters are signed). Here is a patch. Wouldn't just an explicit type cast to `(unsigned char)ch' suffice? -- jan +-- Jan Prikryl| vr|vis center for virtual reality and visualisation [EMAIL PROTECTED] | http://www.vrvis.at +--
Re: wget 1.6 porting problem with snprintf and isdigit on Solaris 2.5.1
Date: Sun, 8 Apr 2001 12:05:35 +0200 From: Jan Prikryl [EMAIL PROTECTED] Wouldn't just an explicit type cast to `(unsigned char)ch' suffice? That would work for now, but it won't work if wget got properly internationalized. That is because isdigit(x) succeeds for non-ASCII digits in some locales. Some locales have multiple ways to represent the decimal digits, and some locales even have non-decimal digits. It's best to use isdigit only when one wants _all_ the characters that are digits, not just '0' through '9'. If you just want '0' through '9', then you should use the test '0' = x x = '9'; this code is guaranteed to work in all locales.
Re: wget 1.6 porting problem with snprintf and isdigit on Solaris 2.5.1
Paul Eggert [EMAIL PROTECTED] writes: That would work for now, but it won't work if wget got properly internationalized. That is because isdigit(x) succeeds for non-ASCII digits in some locales. Some locales have multiple ways to represent the decimal digits, and some locales even have non-decimal digits. It will still work, because Wget doesn't call setlocale() with LC_CTYPE. Therefore it is, as far as is* macros are concerned, in C locale. Not setting the locale for characters is an intentional decision, brought because Wget reads data from the web, so the current locale setting are pretty much irrelevant (and are potentially harmful) for its operation. It's best to use isdigit only when one wants _all_ the characters that are digits, not just '0' through '9'. If you just want '0' through '9', then you should use the test '0' = x x = '9'; this code is guaranteed to work in all locales. In the general case (other is* macros), such hacked-up code is probably slower than table lookups. That's why Wget 1.7 incorporates its own implementation of IS* macros (from Gcc) and uses them consistently. Except in snprintf.c, which is not part of Wget proper. *sigh*
Re: wget 1.6 porting problem with snprintf and isdigit on Solaris 2.5.1
Russ Allbery [EMAIL PROTECTED] writes: That's why Wget 1.7 incorporates its own implementation of IS* macros (from Gcc) and uses them consistently. Except in snprintf.c, which is not part of Wget proper. *sigh* ...since snprintf.c isn't part of wget proper, it *might* be called in a situation where there are non-ASCII digits, and I bet what it really wants to know is whether something is between 0 and 9 inclusive. So I think a valid argument could be made that this is a bug in the upstream snprintf.c that the author would probably take a patch for (it should probably use Paul's recommended explicit test), which would get rid of the warning. :) The problem is, I'm not sure there is an upstream version of snprintf.c. As you know, it is used by several different projects. That's why I'm trying to avoid changing it, unless it is absolutely necessary. Maybe we should create an snprintf.c mailing list? :-)
Re: wget 1.6 porting problem with snprintf and isdigit on Solaris 2.5.1
Hrvoje Niksic [EMAIL PROTECTED] writes: The problem is, I'm not sure there is an upstream version of snprintf.c. As you know, it is used by several different projects. That's why I'm trying to avoid changing it, unless it is absolutely necessary. I'd been assuming http://www.fiction.net/~blong/programs/#snprintf would serve as the canonical upstream location, but I'm not sure if your last patch made it into that version. Maybe we should create an snprintf.c mailing list? :-) That's probably not that bad of an idea. :) I think the world really needs a good public domain implementation of snprintf, and this one is close enough to public domain to be essentially the same thing. -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/
Re: wget 1.6 porting problem with snprintf and isdigit on Solaris 2.5.1
From: Hrvoje Niksic [EMAIL PROTECTED] Date: 08 Apr 2001 23:06:32 +0200 In the general case (other is* macros), such hacked-up code is probably slower than table lookups. For the special case of isdigit, '0'=x x='9' is usually faster than table lookups. Decent compilers like GCC optimize that expression into the equivalent of (x - '0') = 9u, and subtracting a constant like '0' is typically faster than table lookup. It will still work, because Wget doesn't call setlocale() with LC_CTYPE. Yes, Wget doesn't now (on decent hosts), but I thought it conceivable that it might in the future. Also, on hosts without LC_MESSAGES, Wget 1.6 invokes setlocale with LC_ALL, which in turn affects LC_CTYPE. So I thought it safer (as well as faster) for Wget to use '0'=x x='9'.
