* [Qemu-devel] [PATCH] wrap up use of dangerous ctype.h macros
@ 2008-02-20 16:24 Ian Jackson
2008-08-28 17:21 ` [Qemu-devel] [PATCH] [RESEND] " Ian Jackson
0 siblings, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2008-02-20 16:24 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 1109 bytes --]
The macros isupper, isspace, tolower, and so forth (from <ctype.h>)
are defined to take an int containing the value of an unsigned char,
or EOF.
This means that you mustn't write:
char *p;
...
if (isspace(*p)) { ...
If *p is a top-bit-set character, and your host architecture has
signed chars by default (most do), then the result is passing a
negative number to isspace, which is not allowed. glibc permits this
but not all libcs do, and it is wrong according to C89 and C99.
The correct use is:
if (isspace((unsigned char)*p)) { ...
This is rather unweildy so I in the attached patch have invented a
CTYPE macro which takes some but not all of the pain out of it.
I haven't checked the context of every use I changed to confirm that
the char* is actually a char* rather than an unsigned char*, and I did
not check whether the data is static data which is part of qemu and
guaranteed to contain only 7-bit characters - so technically it is
possible that some of these changes are unnecessary. However they are
all correct changes and changing it everywhere sets the right example.
Ian.
[-- Attachment #2: new CTYPE macro for calling <ctype.h> safely --]
[-- Type: text/plain, Size: 10399 bytes --]
diff --git a/audio/audio.c b/audio/audio.c
index 2ccef58..653e196 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -235,7 +235,7 @@ static char *audio_alloc_prefix (const char *s)
strcat (r, s);
for (i = 0; i < len; ++i) {
- u[i] = toupper (u[i]);
+ u[i] = CTYPE(toupper, u[i]);
}
}
return r;
@@ -489,7 +489,7 @@ static void audio_process_options (const char *prefix,
/* copy while upper-casing, including trailing zero */
for (i = 0; i <= preflen; ++i) {
- optname[i + sizeof (qemu_prefix) - 1] = toupper (prefix[i]);
+ optname[i + sizeof (qemu_prefix) - 1] = CTYPE(toupper, prefix[i]);
}
strcat (optname, "_");
strcat (optname, opt->name);
diff --git a/block-vvfat.c b/block-vvfat.c
index ad81425..7fe7e6a 100644
--- a/block-vvfat.c
+++ b/block-vvfat.c
@@ -1476,7 +1476,7 @@ static int parse_short_name(BDRVVVFATState* s,
if (direntry->name[i] <= ' ' || direntry->name[i] > 0x7f)
return -1;
else if (s->downcase_short_names)
- lfn->name[i] = tolower(direntry->name[i]);
+ lfn->name[i] = CTYPE(tolower,direntry->name[i]);
else
lfn->name[i] = direntry->name[i];
}
@@ -1489,7 +1489,7 @@ static int parse_short_name(BDRVVVFATState* s,
if (direntry->extension[j] <= ' ' || direntry->extension[j] > 0x7f)
return -2;
else if (s->downcase_short_names)
- lfn->name[i + j] = tolower(direntry->extension[j]);
+ lfn->name[i + j] = CTYPE(tolower,direntry->extension[j]);
else
lfn->name[i + j] = direntry->extension[j];
}
diff --git a/cutils.c b/cutils.c
index 9ef2fa6..e8e022c 100644
--- a/cutils.c
+++ b/cutils.c
@@ -72,7 +72,7 @@ int stristart(const char *str, const char *val, const char **ptr)
p = str;
q = val;
while (*q != '\0') {
- if (toupper(*p) != toupper(*q))
+ if (CTYPE(toupper,*p) != CTYPE(toupper,*q))
return 0;
p++;
q++;
diff --git a/monitor.c b/monitor.c
index 63d9fad..8e2187f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1779,7 +1779,7 @@ static void next(void)
{
if (pch != '\0') {
pch++;
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
}
}
@@ -1838,7 +1838,7 @@ static int64_t expr_unary(void)
*q++ = *pch;
pch++;
}
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
*q = 0;
ret = get_monitor_def(®, buf);
@@ -1863,7 +1863,7 @@ static int64_t expr_unary(void)
expr_error("invalid char in expression");
}
pch = p;
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
break;
}
@@ -1957,7 +1957,7 @@ static int get_expr(int64_t *pval, const char **pp)
*pp = pch;
return -1;
}
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
*pval = expr_sum();
*pp = pch;
@@ -1972,7 +1972,7 @@ static int get_str(char *buf, int buf_size, const char **pp)
q = buf;
p = *pp;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '\0') {
fail:
@@ -2017,7 +2017,7 @@ static int get_str(char *buf, int buf_size, const char **pp)
}
p++;
} else {
- while (*p != '\0' && !isspace(*p)) {
+ while (*p != '\0' && !CTYPE(isspace,*p)) {
if ((q - buf) < buf_size - 1) {
*q++ = *p;
}
@@ -2052,12 +2052,12 @@ static void monitor_handle_command(const char *cmdline)
/* extract the command name */
p = cmdline;
q = cmdname;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '\0')
return;
pstart = p;
- while (*p != '\0' && *p != '/' && !isspace(*p))
+ while (*p != '\0' && *p != '/' && !CTYPE(isspace,*p))
p++;
len = p - pstart;
if (len > sizeof(cmdname) - 1)
@@ -2093,7 +2093,7 @@ static void monitor_handle_command(const char *cmdline)
int ret;
char *str;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*typestr == '?') {
typestr++;
@@ -2134,7 +2134,7 @@ static void monitor_handle_command(const char *cmdline)
{
int count, format, size;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '/') {
/* format found */
@@ -2181,7 +2181,7 @@ static void monitor_handle_command(const char *cmdline)
}
}
next:
- if (*p != '\0' && !isspace(*p)) {
+ if (*p != '\0' && !CTYPE(isspace,*p)) {
term_printf("invalid char in format: '%c'\n", *p);
goto fail;
}
@@ -2215,7 +2215,7 @@ static void monitor_handle_command(const char *cmdline)
{
int64_t val;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*typestr == '?' || *typestr == '.') {
if (*typestr == '?') {
@@ -2226,7 +2226,7 @@ static void monitor_handle_command(const char *cmdline)
} else {
if (*p == '.') {
p++;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
has_arg = 1;
} else {
@@ -2271,7 +2271,7 @@ static void monitor_handle_command(const char *cmdline)
c = *typestr++;
if (c == '\0')
goto bad_type;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
has_option = 0;
if (*p == '-') {
@@ -2296,7 +2296,7 @@ static void monitor_handle_command(const char *cmdline)
}
}
/* check that all arguments were parsed */
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p != '\0') {
term_printf("%s: extraneous characters at the end of line\n",
@@ -2434,7 +2434,7 @@ static void parse_cmdline(const char *cmdline,
p = cmdline;
nb_args = 0;
for(;;) {
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '\0')
break;
@@ -2468,7 +2468,7 @@ void readline_find_completion(const char *cmdline)
/* if the line ends with a space, it means we want to complete the
next arg */
len = strlen(cmdline);
- if (len > 0 && isspace(cmdline[len - 1])) {
+ if (len > 0 && CTYPE(isspace,cmdline[len - 1])) {
if (nb_args >= MAX_ARGS)
return;
args[nb_args++] = qemu_strdup("");
diff --git a/qemu-common.h b/qemu-common.h
index e8ea687..9284789 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -83,6 +83,17 @@ int strstart(const char *str, const char *val, const char **ptr);
int stristart(const char *str, const char *val, const char **ptr);
time_t mktimegm(struct tm *tm);
+#define CTYPE(isfoobar,argumentchar) (isfoobar((unsigned char)(argumentchar)))
+ /* One must not pass a plain `char' to isupper, toupper, et al. If
+ * it has the top bit set (ie, is negative if your chars are
+ * signed), undefined behaviour results. The <ctype.h> functions
+ * are defined to take the value of an unsigned char, as an int.
+ * So use this macro. You may pass toupper et al for isfoobar.
+ * Do not pass EOF as a character to this macro. If you might have
+ * EOF then you ought to have it in an int representing an unsigned
+ * char, which is safe for the ctype macros directly. Or test separately.
+ * Obviously don't use this for floating point things like isnan! */
+
/* Error handling. */
void hw_error(const char *fmt, ...)
diff --git a/readline.c b/readline.c
index 81bc491..ad10e1e 100644
--- a/readline.c
+++ b/readline.c
@@ -169,7 +169,7 @@ static void term_backword(void)
/* find first word (backwards) */
while (start > 0) {
- if (!isspace(term_cmd_buf[start])) {
+ if (!CTYPE(isspace,term_cmd_buf[start])) {
break;
}
@@ -178,7 +178,7 @@ static void term_backword(void)
/* find first space (backwards) */
while (start > 0) {
- if (isspace(term_cmd_buf[start])) {
+ if (CTYPE(isspace,term_cmd_buf[start])) {
++start;
break;
}
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ad12364..6fb4d1a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -25,6 +25,7 @@
#include "dis-asm.h"
#include "host-utils.h"
+#include "qemu-common.h"
//#define PPC_DUMP_CPU
//#define PPC_DEBUG_SPR
@@ -9460,7 +9461,7 @@ const ppc_def_t *cpu_ppc_find_by_name (const unsigned char *name)
p = name;
check_pvr:
for (i = 0; i < 8; i++) {
- if (!isxdigit(*p++))
+ if (!CTYPE(isxdigit,*p++))
break;
}
if (i == 8)
diff --git a/usb-linux.c b/usb-linux.c
index d8032a8..7f75598 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -719,7 +719,7 @@ static int get_tag_value(char *buf, int buf_size,
if (!p)
return -1;
p += strlen(tag);
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
q = buf;
while (*p != '\0' && !strchr(stopchars, *p)) {
diff --git a/vl.c b/vl.c
index 15955db..552d4b1 100644
--- a/vl.c
+++ b/vl.c
@@ -3562,7 +3562,7 @@ int parse_host_port(struct sockaddr_in *saddr, const char *str)
if (buf[0] == '\0') {
saddr->sin_addr.s_addr = 0;
} else {
- if (isdigit(buf[0])) {
+ if (CTYPE(isdigit,buf[0])) {
if (!inet_aton(buf, &saddr->sin_addr))
return -1;
} else {
@@ -3964,7 +3964,7 @@ int tap_alloc(char *dev)
if( *dev ){
ptr = dev;
- while( *ptr && !isdigit((int)*ptr) ) ptr++;
+ while( *ptr && !CTYPE(isdigit,(int)*ptr) ) ptr++;
ppa = atoi(ptr);
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH] [RESEND] wrap up use of dangerous ctype.h macros
2008-02-20 16:24 [Qemu-devel] [PATCH] wrap up use of dangerous ctype.h macros Ian Jackson
@ 2008-08-28 17:21 ` Ian Jackson
2008-08-28 20:22 ` Anthony Liguori
0 siblings, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2008-08-28 17:21 UTC (permalink / raw)
To: qemu-devel
The macros isupper, isspace, tolower, and so forth (from <ctype.h>)
are defined to take an int containing the value of an unsigned char,
or EOF.
This means that you mustn't write:
char *p;
...
if (isspace(*p)) { ...
If *p is a top-bit-set character, and your host architecture has
signed chars by default (most do), then the result is passing a
negative number to isspace, which is not allowed. glibc permits this
but not all libcs do, and it is wrong according to C89 and C99.
The correct use is:
if (isspace((unsigned char)*p)) { ...
This is rather unweildy and error-prone so I in the attached patch
have invented a CTYPE macro which takes some but not all of the pain
out of it.
Technically it is possible that some of these changes are unnecessary
as I haven't checked the context of every use I changed to confirm
that the char* is actually a char* rather than an unsigned char*, and
I did not check whether the data is static data which is part of qemu
and guaranteed to contain only 7-bit characters. However they are all
correct changes and changing it everywhere sets the right example.
This change gets rid of compiler warnings on some platforms, too
(generally, correct compiler warnings which are reporting exactly the
problem I am fixing here).
Ian.
Use new CTYPE macro for <ctype.h>
This avoids passing a negative int to isfoobar,
which is not allowed
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
audio/audio.c | 4 ++--
block-vvfat.c | 6 +++---
cutils.c | 2 +-
monitor.c | 38 +++++++++++++++++++-------------------
nbd.c | 16 ++++++++--------
qemu-common.h | 10 ++++++++++
readline.c | 4 ++--
target-ppc/translate_init.c | 3 ++-
usb-linux.c | 2 +-
vl.c | 4 ++--
10 files changed, 50 insertions(+), 39 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 20bb2fc..ba14066 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -215,7 +215,7 @@ static char *audio_alloc_prefix (const char *s)
pstrcat (r, len, s);
for (i = 0; i < len; ++i) {
- u[i] = toupper (u[i]);
+ u[i] = CTYPE(toupper, u[i]);
}
}
return r;
@@ -471,7 +471,7 @@ static void audio_process_options (const char *prefix,
/* copy while upper-casing, including trailing zero */
for (i = 0; i <= preflen; ++i) {
- optname[i + sizeof (qemu_prefix) - 1] = toupper (prefix[i]);
+ optname[i + sizeof (qemu_prefix) - 1] = CTYPE(toupper, prefix[i]);
}
pstrcat (optname, optlen, "_");
optlen--;
diff --git a/block-vvfat.c b/block-vvfat.c
index 79804a7..2b5b0b4 100644
--- a/block-vvfat.c
+++ b/block-vvfat.c
@@ -1052,7 +1052,7 @@ DLOG(if (stderr == NULL) {
i = strrchr(dirname, ':') - dirname;
assert(i >= 3);
- if (dirname[i-2] == ':' && isalpha(dirname[i-1]))
+ if (dirname[i-2] == ':' && CTYPE(isalpha,dirname[i-1]))
/* workaround for DOS drive names */
dirname += i-1;
else
@@ -1481,7 +1481,7 @@ static int parse_short_name(BDRVVVFATState* s,
if (direntry->name[i] <= ' ' || direntry->name[i] > 0x7f)
return -1;
else if (s->downcase_short_names)
- lfn->name[i] = tolower(direntry->name[i]);
+ lfn->name[i] = CTYPE(tolower,direntry->name[i]);
else
lfn->name[i] = direntry->name[i];
}
@@ -1494,7 +1494,7 @@ static int parse_short_name(BDRVVVFATState* s,
if (direntry->extension[j] <= ' ' || direntry->extension[j] > 0x7f)
return -2;
else if (s->downcase_short_names)
- lfn->name[i + j] = tolower(direntry->extension[j]);
+ lfn->name[i + j] = CTYPE(tolower,direntry->extension[j]);
else
lfn->name[i + j] = direntry->extension[j];
}
diff --git a/cutils.c b/cutils.c
index 9ef2fa6..e8e022c 100644
--- a/cutils.c
+++ b/cutils.c
@@ -72,7 +72,7 @@ int stristart(const char *str, const char *val, const char **ptr)
p = str;
q = val;
while (*q != '\0') {
- if (toupper(*p) != toupper(*q))
+ if (CTYPE(toupper,*p) != CTYPE(toupper,*q))
return 0;
p++;
q++;
diff --git a/monitor.c b/monitor.c
index e71f49e..7907246 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1900,7 +1900,7 @@ static void next(void)
{
if (pch != '\0') {
pch++;
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
}
}
@@ -1959,7 +1959,7 @@ static int64_t expr_unary(void)
*q++ = *pch;
pch++;
}
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
*q = 0;
ret = get_monitor_def(®, buf);
@@ -1984,7 +1984,7 @@ static int64_t expr_unary(void)
expr_error("invalid char in expression");
}
pch = p;
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
break;
}
@@ -2078,7 +2078,7 @@ static int get_expr(int64_t *pval, const char **pp)
*pp = pch;
return -1;
}
- while (isspace(*pch))
+ while (CTYPE(isspace,*pch))
pch++;
*pval = expr_sum();
*pp = pch;
@@ -2093,7 +2093,7 @@ static int get_str(char *buf, int buf_size, const char **pp)
q = buf;
p = *pp;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '\0') {
fail:
@@ -2138,7 +2138,7 @@ static int get_str(char *buf, int buf_size, const char **pp)
}
p++;
} else {
- while (*p != '\0' && !isspace(*p)) {
+ while (*p != '\0' && !CTYPE(isspace,*p)) {
if ((q - buf) < buf_size - 1) {
*q++ = *p;
}
@@ -2184,12 +2184,12 @@ static void monitor_handle_command(const char *cmdline)
/* extract the command name */
p = cmdline;
q = cmdname;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '\0')
return;
pstart = p;
- while (*p != '\0' && *p != '/' && !isspace(*p))
+ while (*p != '\0' && *p != '/' && !CTYPE(isspace,*p))
p++;
len = p - pstart;
if (len > sizeof(cmdname) - 1)
@@ -2225,7 +2225,7 @@ static void monitor_handle_command(const char *cmdline)
int ret;
char *str;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*typestr == '?') {
typestr++;
@@ -2266,15 +2266,15 @@ static void monitor_handle_command(const char *cmdline)
{
int count, format, size;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '/') {
/* format found */
p++;
count = 1;
- if (isdigit(*p)) {
+ if (CTYPE(isdigit,*p)) {
count = 0;
- while (isdigit(*p)) {
+ while (CTYPE(isdigit,*p)) {
count = count * 10 + (*p - '0');
p++;
}
@@ -2313,7 +2313,7 @@ static void monitor_handle_command(const char *cmdline)
}
}
next:
- if (*p != '\0' && !isspace(*p)) {
+ if (*p != '\0' && !CTYPE(isspace,*p)) {
term_printf("invalid char in format: '%c'\n", *p);
goto fail;
}
@@ -2347,7 +2347,7 @@ static void monitor_handle_command(const char *cmdline)
{
int64_t val;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*typestr == '?' || *typestr == '.') {
if (*typestr == '?') {
@@ -2358,7 +2358,7 @@ static void monitor_handle_command(const char *cmdline)
} else {
if (*p == '.') {
p++;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
has_arg = 1;
} else {
@@ -2403,7 +2403,7 @@ static void monitor_handle_command(const char *cmdline)
c = *typestr++;
if (c == '\0')
goto bad_type;
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
has_option = 0;
if (*p == '-') {
@@ -2428,7 +2428,7 @@ static void monitor_handle_command(const char *cmdline)
}
}
/* check that all arguments were parsed */
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p != '\0') {
term_printf("%s: extraneous characters at the end of line\n",
@@ -2576,7 +2576,7 @@ static void parse_cmdline(const char *cmdline,
p = cmdline;
nb_args = 0;
for(;;) {
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
if (*p == '\0')
break;
@@ -2610,7 +2610,7 @@ void readline_find_completion(const char *cmdline)
/* if the line ends with a space, it means we want to complete the
next arg */
len = strlen(cmdline);
- if (len > 0 && isspace(cmdline[len - 1])) {
+ if (len > 0 && CTYPE(isspace,cmdline[len - 1])) {
if (nb_args >= MAX_ARGS)
return;
args[nb_args++] = qemu_strdup("");
diff --git a/nbd.c b/nbd.c
index 9bebe4a..5197e15 100644
--- a/nbd.c
+++ b/nbd.c
@@ -310,14 +310,14 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
*blocksize = 1024;
TRACE("Magic is %c%c%c%c%c%c%c%c",
- isprint(buf[0]) ? buf[0] : '.',
- isprint(buf[1]) ? buf[1] : '.',
- isprint(buf[2]) ? buf[2] : '.',
- isprint(buf[3]) ? buf[3] : '.',
- isprint(buf[4]) ? buf[4] : '.',
- isprint(buf[5]) ? buf[5] : '.',
- isprint(buf[6]) ? buf[6] : '.',
- isprint(buf[7]) ? buf[7] : '.');
+ CTYPE(isprint,buf[0]) ? buf[0] : '.',
+ CTYPE(isprint,buf[1]) ? buf[1] : '.',
+ CTYPE(isprint,buf[2]) ? buf[2] : '.',
+ CTYPE(isprint,buf[3]) ? buf[3] : '.',
+ CTYPE(isprint,buf[4]) ? buf[4] : '.',
+ CTYPE(isprint,buf[5]) ? buf[5] : '.',
+ CTYPE(isprint,buf[6]) ? buf[6] : '.',
+ CTYPE(isprint,buf[7]) ? buf[7] : '.');
TRACE("Magic is 0x%" PRIx64, magic);
TRACE("Size is %" PRIu64, *size);
diff --git a/qemu-common.h b/qemu-common.h
index 23d1444..5d5f46b 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -94,6 +94,16 @@ char *qemu_strdup(const char *str);
void *get_mmap_addr(unsigned long size);
+#define CTYPE(isfoobar,argumentchar) (isfoobar((unsigned char)(argumentchar)))
+ /* One must not pass a plain `char' to isupper, toupper, et al. If
+ * it has the top bit set (ie, is negative if your chars are
+ * signed), undefined behaviour results. The <ctype.h> functions
+ * are defined to take the value of an unsigned char, as an int.
+ * So use this macro. You may pass toupper et al for isfoobar.
+ * Do not pass EOF as a character to this macro. If you might have
+ * EOF then you ought to have it in an int representing an unsigned
+ * char, which is safe for the ctype macros directly. Or test separately.
+ * Obviously don't use this for floating point things like isnan! */
/* Error handling. */
diff --git a/readline.c b/readline.c
index 81bc491..ad10e1e 100644
--- a/readline.c
+++ b/readline.c
@@ -169,7 +169,7 @@ static void term_backword(void)
/* find first word (backwards) */
while (start > 0) {
- if (!isspace(term_cmd_buf[start])) {
+ if (!CTYPE(isspace,term_cmd_buf[start])) {
break;
}
@@ -178,7 +178,7 @@ static void term_backword(void)
/* find first space (backwards) */
while (start > 0) {
- if (isspace(term_cmd_buf[start])) {
+ if (CTYPE(isspace,term_cmd_buf[start])) {
++start;
break;
}
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ad12364..6fb4d1a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -25,6 +25,7 @@
#include "dis-asm.h"
#include "host-utils.h"
+#include "qemu-common.h"
//#define PPC_DUMP_CPU
//#define PPC_DEBUG_SPR
@@ -9460,7 +9461,7 @@ const ppc_def_t *cpu_ppc_find_by_name (const unsigned char *name)
p = name;
check_pvr:
for (i = 0; i < 8; i++) {
- if (!isxdigit(*p++))
+ if (!CTYPE(isxdigit,*p++))
break;
}
if (i == 8)
diff --git a/usb-linux.c b/usb-linux.c
index c31d56a..9ed335e 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -703,7 +703,7 @@ static int get_tag_value(char *buf, int buf_size,
if (!p)
return -1;
p += strlen(tag);
- while (isspace(*p))
+ while (CTYPE(isspace,*p))
p++;
q = buf;
while (*p != '\0' && !strchr(stopchars, *p)) {
diff --git a/vl.c b/vl.c
index 245177a..fd4dcc7 100644
--- a/vl.c
+++ b/vl.c
@@ -4003,7 +4003,7 @@ int parse_host_port(struct sockaddr_in *saddr, const char *str)
if (buf[0] == '\0') {
saddr->sin_addr.s_addr = 0;
} else {
- if (isdigit(buf[0])) {
+ if (CTYPE(isdigit,buf[0])) {
if (!inet_aton(buf, &saddr->sin_addr))
return -1;
} else {
@@ -4418,7 +4418,7 @@ int tap_alloc(char *dev, size_t dev_size)
if( *dev ){
ptr = dev;
- while( *ptr && !isdigit((int)*ptr) ) ptr++;
+ while( *ptr && !CTYPE(isdigit,(int)*ptr) ) ptr++;
ppa = atoi(ptr);
}
--
1.4.4.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] [RESEND] wrap up use of dangerous ctype.h macros
2008-08-28 17:21 ` [Qemu-devel] [PATCH] [RESEND] " Ian Jackson
@ 2008-08-28 20:22 ` Anthony Liguori
2008-08-29 9:42 ` Ian Jackson
0 siblings, 1 reply; 5+ messages in thread
From: Anthony Liguori @ 2008-08-28 20:22 UTC (permalink / raw)
To: qemu-devel
Ian Jackson wrote:
> The macros isupper, isspace, tolower, and so forth (from <ctype.h>)
> are defined to take an int containing the value of an unsigned char,
> or EOF.
>
> This means that you mustn't write:
> char *p;
> ...
> if (isspace(*p)) { ...
>
> If *p is a top-bit-set character, and your host architecture has
> signed chars by default (most do), then the result is passing a
> negative number to isspace, which is not allowed. glibc permits this
> but not all libcs do, and it is wrong according to C89 and C99.
>
> The correct use is:
> if (isspace((unsigned char)*p)) { ...
>
> This is rather unweildy and error-prone so I in the attached patch
> have invented a CTYPE macro which takes some but not all of the pain
> out of it.
>
Can't we just have qemu_isspace() macros? I find the CTYPE macro quite
awkward. I don't think most people are comfortable with that kind of
syntax.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] [RESEND] wrap up use of dangerous ctype.h macros
2008-08-28 20:22 ` Anthony Liguori
@ 2008-08-29 9:42 ` Ian Jackson
2008-09-07 2:49 ` Anthony Liguori
0 siblings, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2008-08-29 9:42 UTC (permalink / raw)
To: qemu-devel
Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] [RESEND] wrap up use of dangerous ctype.h macros"):
> Can't we just have qemu_isspace() macros? I find the CTYPE macro quite
> awkward. I don't think most people are comfortable with that kind of
> syntax.
I don't have a strong opinion about this. But of course qemu_isspace
et al would require us to list in qemu-common.h all of the isfoobar
and towombat functions.
Ian.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] [RESEND] wrap up use of dangerous ctype.h macros
2008-08-29 9:42 ` Ian Jackson
@ 2008-09-07 2:49 ` Anthony Liguori
0 siblings, 0 replies; 5+ messages in thread
From: Anthony Liguori @ 2008-09-07 2:49 UTC (permalink / raw)
To: qemu-devel
Ian Jackson wrote:
> Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] [RESEND] wrap up use of dangerous ctype.h macros"):
>
>> Can't we just have qemu_isspace() macros? I find the CTYPE macro quite
>> awkward. I don't think most people are comfortable with that kind of
>> syntax.
>>
>
> I don't have a strong opinion about this. But of course qemu_isspace
> et al would require us to list in qemu-common.h all of the isfoobar
> and towombat functions.
>
At least, the ones we actually use. I'd definitely prefer this.
Regards,
Anthony Liguori
> Ian.
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-09-07 2:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-20 16:24 [Qemu-devel] [PATCH] wrap up use of dangerous ctype.h macros Ian Jackson
2008-08-28 17:21 ` [Qemu-devel] [PATCH] [RESEND] " Ian Jackson
2008-08-28 20:22 ` Anthony Liguori
2008-08-29 9:42 ` Ian Jackson
2008-09-07 2:49 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).