OSSP CVS Repository

ossp - Check-in [1336]
Not logged in
[Honeypot]  [Browse]  [Home]  [Login]  [Reports
[Search]  [Ticket]  [Timeline
  [Patchset]  [Tagging/Branching

Check-in Number: 1336
Date: 2001-Nov-14 13:59:24 (local)
2001-Nov-14 12:59:24 (UTC)
User:rse
Branch:
Comment: remember my stuff here (added strerror issue, too)
Tickets:
Inspections:
Files:
ossp-pkg/var/TODO      1.4 -> 1.5     98 inserted, 0 deleted

ossp-pkg/var/TODO 1.4 -> 1.5

--- 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?
+
+------------------------------------------------------------------------
+

CVSTrac 2.0.1