From c1ee35af3b67ab70d3a1fd6d47500621a2eaaab0 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Fri, 16 May 2008 09:37:44 +0000 Subject: [PATCH] start using c-ctype functions Up to now, we've been avoiding ctype functions like isspace, isdigit, etc. because they are locale-dependent. Now that we have the c-ctype functions, we can start using *them*, to make the code more readable with changes like these: - /* This may not work on EBCDIC. */ - if ((*p >= 'a' && *p <= 'z') || - (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9')) + if (c_isalnum(*p)) - while ((*cur >= '0') && (*cur <= '9')) { + while (c_isdigit(*cur)) { Also, some macros in conf.c used names that conflicted with standard meaning of "BLANK" and "SPACE", so I've adjusted them to be in line with the definition of e.g., isblank. In addition, I've wrapped those statement macros with do {...} while (0), so that we can't forget the ";" after a use. There was one like that already (fixed below). The missing semicolon would mess up automatic indenting. * src/buf.c (virBufferURIEncodeString): * src/conf.c (IS_EOL, SKIP_BLANKS_AND_EOL, SKIP_BLANKS) (virConfParseLong, virConfParseValue, virConfParseName) (virConfParseSeparator, virConfParseStatement, IS_BLANK, IS_CHAR) (IS_DIGIT, IS_SPACE, SKIP_SPACES): * src/nodeinfo.c: * src/qemu_conf.c (qemudParseInterfaceXML): * src/qemu_driver.c (qemudDomainBlockStats): * src/sexpr.c: * src/stats_linux.c: * src/util.c (virParseNumber, virDiskNameToIndex): * src/uuid.c (hextobin, virUUIDParse): * src/virsh.c: * src/xml.c (parseCpuNumber, virParseCpuSet): --- ChangeLog | 37 +++++++++++++++++++++++++++++++ src/buf.c | 11 +++------- src/conf.c | 56 +++++++++++++++++++++++------------------------ src/nodeinfo.c | 2 +- src/qemu_conf.c | 10 ++++----- src/qemu_driver.c | 6 ++--- src/sexpr.c | 2 +- src/stats_linux.c | 2 +- src/util.c | 6 ++--- src/uuid.c | 35 +++++++++++++++++------------ src/virsh.c | 2 +- src/xml.c | 6 ++--- 12 files changed, 105 insertions(+), 70 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1d524f4682..6c6a84ce4b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,42 @@ Fri May 16 11:29:30 CEST 2008 Jim Meyering + start using c-ctype functions + Up to now, we've been avoiding ctype functions like isspace, isdigit, + etc. because they are locale-dependent. Now that we have the c-ctype + functions, we can start using *them*, to make the code more readable + with changes like these: + + - /* This may not work on EBCDIC. */ + - if ((*p >= 'a' && *p <= 'z') || + - (*p >= 'A' && *p <= 'Z') || + - (*p >= '0' && *p <= '9')) + + if (c_isalnum(*p)) + + - while ((*cur >= '0') && (*cur <= '9')) { + + while (c_isdigit(*cur)) { + + Also, some macros in conf.c used names that conflicted with + standard meaning of "BLANK" and "SPACE", so I've adjusted them + to be in line with the definition of e.g., isblank. + In addition, I've wrapped those statement macros with do {...} while (0), + so that we can't forget the ";" after a use. There was one like that + already (fixed below). The missing semicolon would mess up automatic + indenting. + * src/buf.c (virBufferURIEncodeString): + * src/conf.c (IS_EOL, SKIP_BLANKS_AND_EOL, SKIP_BLANKS) + (virConfParseLong, virConfParseValue, virConfParseName) + (virConfParseSeparator, virConfParseStatement, IS_BLANK, IS_CHAR) + (IS_DIGIT, IS_SPACE, SKIP_SPACES): + * src/nodeinfo.c: + * src/qemu_conf.c (qemudParseInterfaceXML): + * src/qemu_driver.c (qemudDomainBlockStats): + * src/sexpr.c: + * src/stats_linux.c: + * src/util.c (virParseNumber, virDiskNameToIndex): + * src/uuid.c (hextobin, virUUIDParse): + * src/virsh.c: + * src/xml.c (parseCpuNumber, virParseCpuSet): + avoid a double-free bug * src/qemu_conf.c (qemudParseXML): Ensure that "obj" is either NULL or a valid malloc'd pointer before we might "goto error" diff --git a/src/buf.c b/src/buf.c index b56a9c18d8..16afaca977 100644 --- a/src/buf.c +++ b/src/buf.c @@ -16,6 +16,7 @@ #include #include #include +#include "c-ctype.h" #define __VIR_BUFFER_C__ @@ -349,10 +350,7 @@ virBufferURIEncodeString (virBufferPtr buf, const char *str) return; for (p = str; *p; ++p) { - /* This may not work on EBCDIC. */ - if ((*p >= 'a' && *p <= 'z') || - (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9')) + if (c_isalnum(*p)) grow_size++; else grow_size += 3; /* %ab */ @@ -362,10 +360,7 @@ virBufferURIEncodeString (virBufferPtr buf, const char *str) return; for (p = str; *p; ++p) { - /* This may not work on EBCDIC. */ - if ((*p >= 'a' && *p <= 'z') || - (*p >= 'A' && *p <= 'Z') || - (*p >= '0' && *p <= '9')) + if (c_isalnum(*p)) buf->content[buf->use++] = *p; else { uc = (unsigned char) *p; diff --git a/src/conf.c b/src/conf.c index e52d8d16a6..6ac480a0cd 100644 --- a/src/conf.c +++ b/src/conf.c @@ -22,6 +22,7 @@ #include "buf.h" #include "conf.h" #include "util.h" +#include "c-ctype.h" /************************************************************************ * * @@ -45,17 +46,14 @@ struct _virConfParserCtxt { #define CUR (*ctxt->cur) #define NEXT if (ctxt->cur < ctxt->end) ctxt->cur++; #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) -#define IS_BLANK(c) (((c) == ' ') || ((c) == '\n') || ((c) == '\r') || \ - ((c) == '\t')) -#define SKIP_BLANKS {while ((ctxt->cur < ctxt->end) && (IS_BLANK(CUR))){\ - if (CUR == '\n') ctxt->line++; \ - ctxt->cur++;}} -#define IS_SPACE(c) (((c) == ' ') || ((c) == '\t')) -#define SKIP_SPACES {while ((ctxt->cur < ctxt->end) && (IS_SPACE(CUR))) \ - ctxt->cur++;} -#define IS_CHAR(c) ((((c) >= 'a') && ((c) <= 'z')) || \ - (((c) >= 'A') && ((c) <= 'Z'))) -#define IS_DIGIT(c) (((c) >= '0') && ((c) <= '9')) + +#define SKIP_BLANKS_AND_EOL \ + do { while ((ctxt->cur < ctxt->end) && (c_isblank(CUR) || IS_EOL(CUR))) { \ + if (CUR == '\n') ctxt->line++; \ + ctxt->cur++;}} while (0) +#define SKIP_BLANKS \ + do { while ((ctxt->cur < ctxt->end) && (c_isblank(CUR))) \ + ctxt->cur++; } while (0) /************************************************************************ * * @@ -338,12 +336,12 @@ virConfParseLong(virConfParserCtxtPtr ctxt, long *val) } else if (CUR == '+') { NEXT; } - if ((ctxt->cur >= ctxt->end) || (!IS_DIGIT(CUR))) { + if ((ctxt->cur >= ctxt->end) || (!c_isdigit(CUR))) { virConfError(NULL, VIR_ERR_CONF_SYNTAX, _("unterminated number"), ctxt->line); return(-1); } - while ((ctxt->cur < ctxt->end) && (IS_DIGIT(CUR))) { + while ((ctxt->cur < ctxt->end) && (c_isdigit(CUR))) { l = l * 10 + (CUR - '0'); NEXT; } @@ -428,7 +426,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt) char *str = NULL; long l = 0; - SKIP_SPACES; + SKIP_BLANKS; if (ctxt->cur >= ctxt->end) { virConfError(NULL, VIR_ERR_CONF_SYNTAX, _("expecting a value"), ctxt->line); @@ -442,10 +440,10 @@ virConfParseValue(virConfParserCtxtPtr ctxt) } else if (CUR == '[') { type = VIR_CONF_LIST; NEXT; - SKIP_BLANKS; + SKIP_BLANKS_AND_EOL; if ((ctxt->cur < ctxt->end) && (CUR != ']')) { lst = virConfParseValue(ctxt); - SKIP_BLANKS; + SKIP_BLANKS_AND_EOL; } while ((ctxt->cur < ctxt->end) && (CUR != ']')) { if (CUR != ',') { @@ -455,7 +453,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt) return(NULL); } NEXT; - SKIP_BLANKS; + SKIP_BLANKS_AND_EOL; if (CUR == ']') { break; } @@ -467,7 +465,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt) prev = lst; while (prev->next != NULL) prev = prev->next; prev->next = tmp; - SKIP_BLANKS; + SKIP_BLANKS_AND_EOL; } if (CUR == ']') { NEXT; @@ -477,7 +475,7 @@ virConfParseValue(virConfParserCtxtPtr ctxt) virConfFreeList(lst); return(NULL); } - } else if (IS_DIGIT(CUR) || (CUR == '-') || (CUR == '+')) { + } else if (c_isdigit(CUR) || (CUR == '-') || (CUR == '+')) { if (virConfParseLong(ctxt, &l) < 0) { return(NULL); } @@ -514,14 +512,14 @@ virConfParseName(virConfParserCtxtPtr ctxt) const char *base; char *ret; - SKIP_SPACES; + SKIP_BLANKS; base = ctxt->cur; /* TODO: probably need encoding support and UTF-8 parsing ! */ - if (!IS_CHAR(CUR)) { + if (!c_isalpha(CUR)) { virConfError(NULL, VIR_ERR_CONF_SYNTAX, _("expecting a name"), ctxt->line); return(NULL); } - while ((ctxt->cur < ctxt->end) && ((IS_CHAR(CUR)) || (IS_DIGIT(CUR)) || (CUR == '_'))) + while ((ctxt->cur < ctxt->end) && (c_isalnum(CUR) || (CUR == '_'))) NEXT; ret = strndup(base, ctxt->cur - base); if (ret == NULL) { @@ -572,14 +570,14 @@ virConfParseComment(virConfParserCtxtPtr ctxt) static int virConfParseSeparator(virConfParserCtxtPtr ctxt) { - SKIP_SPACES; + SKIP_BLANKS; if (ctxt->cur >= ctxt->end) return(0); if (IS_EOL(CUR)) { - SKIP_BLANKS + SKIP_BLANKS_AND_EOL; } else if (CUR == ';') { NEXT; - SKIP_BLANKS; + SKIP_BLANKS_AND_EOL; } else { virConfError(NULL, VIR_ERR_CONF_SYNTAX, _("expecting a separator"), ctxt->line); @@ -604,27 +602,27 @@ virConfParseStatement(virConfParserCtxtPtr ctxt) virConfValuePtr value; char *comm = NULL; - SKIP_BLANKS; + SKIP_BLANKS_AND_EOL; if (CUR == '#') { return(virConfParseComment(ctxt)); } name = virConfParseName(ctxt); if (name == NULL) return(-1); - SKIP_SPACES; + SKIP_BLANKS; if (CUR != '=') { virConfError(NULL, VIR_ERR_CONF_SYNTAX, _("expecting an assignment"), ctxt->line); return(-1); } NEXT; - SKIP_SPACES; + SKIP_BLANKS; value = virConfParseValue(ctxt); if (value == NULL) { free(name); return(-1); } - SKIP_SPACES; + SKIP_BLANKS; if (CUR == '#') { NEXT; base = ctxt->cur; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index b2ef6ee66e..6c303ed99f 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -27,7 +27,7 @@ #include #include #include -#include +#include "c-ctype.h" #ifdef HAVE_SYS_UTSNAME_H #include diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 1a7ab46350..ff7c63e89d 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -49,7 +49,8 @@ #include "buf.h" #include "conf.h" #include "util.h" -#include +#include "verify.h" +#include "c-ctype.h" #define qemudLog(level, msg...) fprintf(stderr, msg) @@ -1007,7 +1008,7 @@ static int qemudParseInterfaceXML(virConnectPtr conn, * i82551 i82557b i82559er ne2k_pci pcnet rtl8139 e1000 virtio */ if (model != NULL) { - int i, len, char_ok; + int i, len; len = xmlStrlen (model); if (len >= QEMUD_MODEL_MAX_LEN) { @@ -1016,10 +1017,7 @@ static int qemudParseInterfaceXML(virConnectPtr conn, goto error; } for (i = 0; i < len; ++i) { - char_ok = - (model[i] >= '0' && model[i] <= '9') || - (model[i] >= 'a' && model[i] <= 'z') || - (model[i] >= 'A' && model[i] <= 'Z') || model[i] == '_'; + int char_ok = c_isalnum(model[i]) || model[i] == '_'; if (!char_ok) { qemudReportError (conn, NULL, NULL, VIR_ERR_INVALID_ARG, "%s", _("Model name contains invalid characters")); diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 34193bd235..1744751fe3 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -49,6 +48,7 @@ #include "libvirt/virterror.h" +#include "c-ctype.h" #include "event.h" #include "buf.h" #include "util.h" @@ -2650,12 +2650,12 @@ qemudDomainBlockStats (virDomainPtr dom, * cdrom to ide1-cd0 * fd[a-] to floppy[0-] */ - if (STRPREFIX (path, "hd") && path[2] >= 'a' && path[2] <= 'z') + if (STRPREFIX (path, "hd") && c_islower(path[2])) snprintf (qemu_dev_name, sizeof (qemu_dev_name), "ide0-hd%d", path[2] - 'a'); else if (STREQ (path, "cdrom")) strcpy (qemu_dev_name, "ide1-cd0"); - else if (STRPREFIX (path, "fd") && path[2] >= 'a' && path[2] <= 'z') + else if (STRPREFIX (path, "fd") && c_islower(path[2])) snprintf (qemu_dev_name, sizeof (qemu_dev_name), "floppy%d", path[2] - 'a'); else { diff --git a/src/sexpr.c b/src/sexpr.c index c275ee28b7..44d9c74c50 100644 --- a/src/sexpr.c +++ b/src/sexpr.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include "c-ctype.h" #include #include "internal.h" diff --git a/src/stats_linux.c b/src/stats_linux.c index 30a4990fb9..4f32d65e95 100644 --- a/src/stats_linux.c +++ b/src/stats_linux.c @@ -18,7 +18,7 @@ #include #include #include -#include +#include "c-ctype.h" #ifdef WITH_XEN #include diff --git a/src/util.c b/src/util.c index 87a46cb1cf..bc39f9f225 100644 --- a/src/util.c +++ b/src/util.c @@ -37,7 +37,7 @@ #include #endif #include -#include +#include "c-ctype.h" #ifdef HAVE_PATHS_H #include @@ -681,7 +681,7 @@ virParseNumber(const char **str) if ((*cur < '0') || (*cur > '9')) return (-1); - while ((*cur >= '0') && (*cur <= '9')) { + while (c_isdigit(*cur)) { unsigned int c = *cur - '0'; if ((ret > INT_MAX / 10) || @@ -794,7 +794,7 @@ int virDiskNameToIndex(const char *name) { while (*ptr) { idx = idx * 26; - if ('a' > *ptr || 'z' < *ptr) + if (!c_islower(*ptr)) return -1; idx += *ptr - 'a'; diff --git a/src/uuid.c b/src/uuid.c index fea1764344..301479b195 100644 --- a/src/uuid.c +++ b/src/uuid.c @@ -32,6 +32,7 @@ #include #include +#include "c-ctype.h" #include "internal.h" #define qemudLog(level, msg...) fprintf(stderr, msg) @@ -105,6 +106,22 @@ virUUIDGenerate(unsigned char *uuid) return virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN); } +/* Convert C from hexadecimal character to integer. */ +static int +hextobin (unsigned char c) +{ + switch (c) + { + default: return c - '0'; + case 'a': case 'A': return 10; + case 'b': case 'B': return 11; + case 'c': case 'C': return 12; + case 'd': case 'D': return 13; + case 'e': case 'E': return 14; + case 'f': case 'F': return 15; + } +} + /** * virUUIDParse: * @uuidstr: zero terminated string representation of the UUID @@ -136,26 +153,16 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) { cur++; continue; } - if ((*cur >= '0') && (*cur <= '9')) - uuid[i] = *cur - '0'; - else if ((*cur >= 'a') && (*cur <= 'f')) - uuid[i] = *cur - 'a' + 10; - else if ((*cur >= 'A') && (*cur <= 'F')) - uuid[i] = *cur - 'A' + 10; - else + if (!c_isxdigit(*cur)) goto error; + uuid[i] = hextobin(*cur); uuid[i] *= 16; cur++; if (*cur == 0) goto error; - if ((*cur >= '0') && (*cur <= '9')) - uuid[i] += *cur - '0'; - else if ((*cur >= 'a') && (*cur <= 'f')) - uuid[i] += *cur - 'a' + 10; - else if ((*cur >= 'A') && (*cur <= 'F')) - uuid[i] += *cur - 'A' + 10; - else + if (!c_isxdigit(*cur)) goto error; + uuid[i] += hextobin(*cur); i++; cur++; } diff --git a/src/virsh.c b/src/virsh.c index af2f1a4fc1..45af630800 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -26,7 +26,7 @@ #include #include #include -#include +#include "c-ctype.h" #include #include #include diff --git a/src/xml.c b/src/xml.c index 22dc2112c2..14b8b70a21 100644 --- a/src/xml.c +++ b/src/xml.c @@ -76,10 +76,10 @@ parseCpuNumber(const char **str, int maxcpu) int ret = 0; const char *cur = *str; - if ((*cur < '0') || (*cur > '9')) + if (!c_isdigit(*cur)) return (-1); - while ((*cur >= '0') && (*cur <= '9')) { + while (c_isdigit(*cur)) { ret = ret * 10 + (*cur - '0'); if (ret >= maxcpu) return (-1); @@ -197,7 +197,7 @@ virParseCpuSet(virConnectPtr conn, const char **str, char sep, neg = 1; } - if ((*cur < '0') || (*cur > '9')) + if (!c_isdigit(*cur)) goto parse_error; start = parseCpuNumber(&cur, maxcpu); if (start < 0) -- 2.47.2