Hello Eric, Pádraig, On 07/15/2014 12:33 AM, Eric Blake wrote:
Here's some real errors:
<...>
Ouch. isdigit() is fairly safe (because POSIX limits it to returning true for exactly 10 bytes that can't become negative when promoted to int), but isblank((int)char) is an absolute bug - there are locales where isblank(255) != isblank((int)(char)255), because char is signed and the value promotes to -1 which is indistinguishable from EOF. Are you sure this code shouldn't be using gnulib's c_isdgit and c_isblank instead? And if you DO want locale comparisons, then use isblank (to_uchar (**endptr)).
Attached is a patch to replace isdigit/isblank with c_isdigit/c_isblank in numfmt. Since numfmt currently does not handle multibyte characters, it is simpler (and quicker for this release) to force the C locale calls, as you suggested. I hope in the future to add true multibyte support. Regards, -gordon
>From 5b5d541dfc744184c3faa41f7d21bb2cc30de88e Mon Sep 17 00:00:00 2001 From: "A. Gordon" <[email protected]> Date: Tue, 15 Jul 2014 12:25:03 -0400 Subject: [PATCH] numfmt: use c-locale comparisons * src/numfmt.c: replace isdigit(),isblank() with c_isdigit(),c_isblank() - as they are safer for non-locale comparisons. Triggered a compilation warning on Cygwin, as explained here: http://lists.gnu.org/archive/html/coreutils/2014-07/msg00087.html --- src/numfmt.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/numfmt.c b/src/numfmt.c index 6091bb6..94a9d7a 100644 --- a/src/numfmt.c +++ b/src/numfmt.c @@ -23,6 +23,7 @@ #include "mbsalign.h" #include "argmatch.h" +#include "c-ctype.h" #include "error.h" #include "quote.h" #include "system.h" @@ -454,7 +455,7 @@ simple_strtod_int (const char *input_str, *negative = false; *endptr = (char *) input_str; - while (*endptr && isdigit (**endptr)) + while (*endptr && c_isdigit (**endptr)) { int digit = (**endptr) - '0'; @@ -600,7 +601,7 @@ simple_strtod_human (const char *input_str, /* process suffix. */ /* Skip any blanks between the number and suffix. */ - while (isblank (**endptr)) + while (c_isblank (**endptr)) (*endptr)++; if (!valid_suffix (**endptr)) @@ -1174,7 +1175,7 @@ process_suffixed_number (char *text, long double *result, size_t *precision) /* Skip white space - always. */ char *p = text; - while (*p && isblank (*p)) + while (*p && c_isblank (*p)) ++p; const unsigned int skip_count = text - p; @@ -1229,9 +1230,9 @@ skip_fields (char *buf, int fields) else while (*ptr && fields--) { - while (*ptr && isblank (*ptr)) + while (*ptr && c_isblank (*ptr)) ++ptr; - while (*ptr && !isblank (*ptr)) + while (*ptr && !c_isblank (*ptr)) ++ptr; } return ptr; -- 1.9.1
