Index: ossp-pkg/var/TODO RCS File: /v/ossp/cvs/ossp-pkg/var/Attic/TODO,v rcsdiff -q -kk '-r1.4' '-r1.5' -u '/v/ossp/cvs/ossp-pkg/var/Attic/TODO,v' 2>/dev/null --- TODO 2001/11/13 14:43:43 1.4 +++ TODO 2001/11/14 12:59:24 1.5 @@ -4,3 +4,101 @@ Regexp-Ausdrücken enthalten. - Unterstützung von PCRE. + + +---- RSE: -------------------------------------------------------------------- + +- HAS TO BE: adding a var_error() function which translates var_rc_t + into a text version ala strerror(3). + +- HAS TO BE: still missing PCRE support plus corresponding Autoconf stuff + +- CAN BE: as in L2, reduce the large 1024 auto-variable with a a lot + smaller (on 32-bit boxes just 21 bytes) but still fully + sufficiently-sized variable: + +Index: var.c +=================================================================== +RCS file: /u/rse/arc/cvs/ossp/ossp-pkg/var/var.c,v +retrieving revision 1.14 +diff -u -d -u -d -r1.14 var.c +--- var.c 2001/11/14 11:11:01 1.14 ++++ var.c 2001/11/14 11:37:54 +@@ -1261,8 +1261,8 @@ + + case '#': /* Substitute length of the string. */ + if (data->begin) { +- char buf[1024]; +- sprintf(buf, "%d", data->end - data->begin); ++ char buf[((sizeof(int)*8)/3)+10]; /* sufficient size: <#bits> x log_10(2) + safety */ ++ sprintf(buf, "%d", (int)(data->end - data->begin)); + tokenbuf_free(data); + if (!tokenbuf_assign(data, buf, strlen(buf))) { + rc = VAR_ERR_OUT_OF_MEMORY; + +- SHOULD BE: the "magic" number 256 in name class definitions would be + nice if replaced by a symbolic name + +- SHOULD BE: the expand_octal() function checks that in the octal number + NNN (representing a character) the first number is not larger than + 3 because the 0xff is (octal) 377. Unfortunately this still allows + things like 399 which is illegal because larger than 0xff (the maximum + fitting into an 8-bit character). My suggestions: + +Index: var.c +=================================================================== +RCS file: /u/rse/arc/cvs/ossp/ossp-pkg/var/var.c,v +retrieving revision 1.15 +diff -u -d -u -d -r1.15 var.c +--- var.c 2001/11/14 12:02:40 1.15 ++++ var.c 2001/11/14 12:14:49 +@@ -209,7 +209,7 @@ + + static var_rc_t expand_octal(const char **src, char **dst, const char *end) + { +- unsigned char c; ++ unsigned int c; + + if (end - *src < 3) + return VAR_ERR_INCOMPLETE_OCTAL; +@@ -217,8 +217,6 @@ + return VAR_ERR_INVALID_OCTAL; + + c = **src - '0'; +- if (c > 3) +- return VAR_ERR_OCTAL_TOO_LARGE; + c *= 8; + ++(*src); + +@@ -227,6 +225,9 @@ + ++(*src); + + c += **src - '0'; ++ ++ if (c > 0xff) ++ return VAR_ERR_OCTAL_TOO_LARGE; + + **dst = (char) c; + ++(*dst); + +- SHOULD BE: the tokenbuf functions like tokenbuf_append() return + 1 or 0 and the other routines convert this into var_rc_t values. I + recommend to change the tokenbuf functions to already return var_rc_t + values and pass this value through in other functions. This way the + different errors in tokenbuf_append() are visible, too. + +- CAN BE: I IMHO would reduce the code size of the recursive descent parser + by 1. making the prototype and corresponding parameter names of all + functions identical (see for example text() for a current exclusion) + and then use a macro to replace the long prototype description on + every function. + +- SHOULD BE: the return code semantics are still not clear to me + (perhaps it is solved by documentation). Especially it confuses me a lot + that internally it uses "int" instead of "var_rc_t" and does arithmetics + like "1 + rc" before finanlly it is casted to an var_rc_t. So how should + anyone can do anything reasonable with return codes > 0. He does not + know what they mean, doesn't he? + +------------------------------------------------------------------------ +