ovsdb: Drop regular expression constraints.
authorBen Pfaff <blp@nicira.com>
Tue, 23 Feb 2010 23:57:50 +0000 (15:57 -0800)
committerBen Pfaff <blp@nicira.com>
Thu, 25 Feb 2010 22:59:16 +0000 (14:59 -0800)
Regular expression constraints have caused nothing but trouble due to the
lack of a ubiquitous regular expression library.  PCRE is *almost*
everywhere, but it has different versions, and different features, and
different bugs, in different places.  It is more trouble than it is worth.
So this commit drops support.

lib/ovsdb-data.c
lib/ovsdb-types.c
lib/ovsdb-types.h
ovsdb/SPECS
ovsdb/ovsdb-idlc.in
tests/ovs-vsctl.at
tests/ovsdb-data.at
tests/ovsdb-types.at
vswitchd/vswitch.ovsschema

index 08d623a..9d8eab8 100644 (file)
@@ -577,30 +577,6 @@ check_string_constraints(const char *s,
             "length %u", s, n_chars, c->maxLen);
     }
 
-#if HAVE_PCRE
-    if (c->re) {
-        int retval;
-
-        retval = pcre_exec(c->re, NULL, s, strlen(s), 0,
-                           PCRE_ANCHORED | PCRE_NO_UTF8_CHECK, NULL, 0);
-        if (retval == PCRE_ERROR_NOMATCH) {
-            if (c->reComment) {
-                return ovsdb_error("constraint violation",
-                                   "\"%s\" is not a %s", s, c->reComment);
-            } else {
-                return ovsdb_error("constraint violation",
-                                   "\"%s\" does not match regular expression "
-                                   "/%s/", s, c->reMatch);
-            }
-        } else if (retval < 0) {
-            /* PCRE doesn't have a function to translate an error code to a
-             * description.  Bizarre.  See pcreapi(3) for error details. */
-            return ovsdb_error("internal error", "PCRE returned error %d",
-                               retval);
-        }
-    }
-#endif  /* HAVE_PCRE */
-
     return NULL;
 }
 
index 5b7e7d8..5da7145 100644 (file)
@@ -134,11 +134,6 @@ ovsdb_base_type_init(struct ovsdb_base_type *base, enum ovsdb_atomic_type type)
         break;
 
     case OVSDB_TYPE_STRING:
-#ifdef HAVE_PCRE
-        base->u.string.re = NULL;
-#endif
-        base->u.string.reMatch = NULL;
-        base->u.string.reComment = NULL;
         base->u.string.minLen = 0;
         base->u.string.maxLen = UINT_MAX;
         break;
@@ -170,21 +165,8 @@ ovsdb_base_type_clone(struct ovsdb_base_type *dst,
         break;
 
     case OVSDB_TYPE_STRING:
-#if HAVE_PCRE
-        if (dst->u.string.re) {
-            pcre_refcount(dst->u.string.re, 1);
-        }
-#else
-        if (dst->u.string.reMatch) {
-            dst->u.string.reMatch = xstrdup(dst->u.string.reMatch);
-        }
-        if (dst->u.string.reComment) {
-            dst->u.string.reComment = xstrdup(dst->u.string.reComment);
-        }
-#endif
         break;
 
-
     case OVSDB_TYPE_UUID:
         if (dst->u.uuid.refTableName) {
             dst->u.uuid.refTableName = xstrdup(dst->u.uuid.refTableName);
@@ -209,16 +191,6 @@ ovsdb_base_type_destroy(struct ovsdb_base_type *base)
             break;
 
         case OVSDB_TYPE_STRING:
-#ifdef HAVE_PCRE
-            if (base->u.string.re && !pcre_refcount(base->u.string.re, -1)) {
-                pcre_free(base->u.string.re);
-                free(base->u.string.reMatch);
-                free(base->u.string.reComment);
-            }
-#else
-            free(base->u.string.reMatch);
-            free(base->u.string.reComment);
-#endif
             break;
 
         case OVSDB_TYPE_UUID:
@@ -281,9 +253,7 @@ ovsdb_base_type_has_constraints(const struct ovsdb_base_type *base)
         return false;
 
     case OVSDB_TYPE_STRING:
-        return (base->u.string.reMatch != NULL
-                || base->u.string.minLen != 0
-                || base->u.string.maxLen != UINT_MAX);
+        return base->u.string.minLen != 0 || base->u.string.maxLen != UINT_MAX;
 
     case OVSDB_TYPE_UUID:
         return base->u.uuid.refTableName != NULL;
@@ -304,44 +274,6 @@ ovsdb_base_type_clear_constraints(struct ovsdb_base_type *base)
     ovsdb_base_type_init(base, type);
 }
 
-struct ovsdb_error *
-ovsdb_base_type_set_regex(struct ovsdb_base_type *base,
-                          const char *reMatch, const char *reComment)
-{
-#ifdef HAVE_PCRE
-    const char *errorString;
-    const char *pattern;
-    int errorOffset;
-
-    /* Compile pattern, anchoring it at both ends. */
-    pattern = reMatch;
-    if (pattern[0] == '\0' || strchr(pattern, '\0')[-1] != '$') {
-        pattern = xasprintf("%s$", pattern);
-    }
-
-#ifndef PCRE_JAVASCRIPT_COMPAT  /* Added in PCRE 7.7. */
-#define PCRE_JAVASCRIPT_COMPAT 0
-#endif
-    base->u.string.re = pcre_compile(pattern, (PCRE_ANCHORED | PCRE_UTF8
-                                               | PCRE_JAVASCRIPT_COMPAT),
-                                     &errorString, &errorOffset, NULL);
-    if (pattern != reMatch) {
-        free((char *) pattern);
-    }
-    if (!base->u.string.re) {
-        return ovsdb_syntax_error(NULL, "invalid regular expression",
-                                  "\"%s\" is not a valid regular "
-                                  "expression: %s", reMatch, errorString);
-    }
-    pcre_refcount(base->u.string.re, 1);
-#endif
-
-    /* Save regular expression. */
-    base->u.string.reMatch = xstrdup(reMatch);
-    base->u.string.reComment = reComment ? xstrdup(reComment) : NULL;
-    return NULL;
-}
-
 static struct ovsdb_error *
 parse_optional_uint(struct ovsdb_parser *parser, const char *member,
                     unsigned int *uint)
@@ -414,20 +346,6 @@ ovsdb_base_type_from_json(struct ovsdb_base_type *base,
             error = ovsdb_syntax_error(json, NULL, "minReal exceeds maxReal");
         }
     } else if (base->type == OVSDB_TYPE_STRING) {
-        const struct json *reMatch;
-
-        reMatch = ovsdb_parser_member(&parser, "reMatch",
-                                      OP_STRING | OP_OPTIONAL);
-        if (reMatch) {
-            const struct json *reComment;
-
-            reComment = ovsdb_parser_member(&parser, "reComment",
-                                            OP_STRING | OP_OPTIONAL);
-            error = ovsdb_base_type_set_regex(
-                base, json_string(reMatch),
-                reComment ? json_string(reComment) : NULL);
-        }
-
         if (!error) {
             error = parse_optional_uint(&parser, "minLength",
                                         &base->u.string.minLen);
@@ -507,13 +425,6 @@ ovsdb_base_type_to_json(const struct ovsdb_base_type *base)
         break;
 
     case OVSDB_TYPE_STRING:
-        if (base->u.string.reMatch) {
-            json_object_put_string(json, "reMatch", base->u.string.reMatch);
-            if (base->u.string.reComment) {
-                json_object_put_string(json, "reComment",
-                                       base->u.string.reComment);
-            }
-        }
         if (base->u.string.minLen != 0) {
             json_object_put(json, "minLength",
                             json_integer_create(base->u.string.minLen));
index 0eca931..0ef596a 100644 (file)
 #include "compiler.h"
 #include "uuid.h"
 
-#ifdef HAVE_PCRE
-#include <pcre.h>
-#endif
-
 struct json;
 
 /* An atomic type: one that OVSDB regards as a single unit of data. */
@@ -64,11 +60,6 @@ struct ovsdb_base_type {
         /* No constraints for Boolean types. */
 
         struct ovsdb_string_constraints {
-#ifdef HAVE_PCRE
-            pcre *re;           /* Compiled regular expression. */
-#endif
-            char *reMatch;      /* reMatch or NULL. */
-            char *reComment;    /* reComment or NULL. */
             unsigned int minLen; /* minLength or 0. */
             unsigned int maxLen; /* maxLength or UINT_MAX. */
         } string;
@@ -86,15 +77,8 @@ struct ovsdb_base_type {
 #define OVSDB_BASE_REAL_INIT    { .type = OVSDB_TYPE_REAL,          \
                                   .u.real = { -DBL_MAX, DBL_MAX } }
 #define OVSDB_BASE_BOOLEAN_INIT { .type = OVSDB_TYPE_BOOLEAN }
-#ifdef HAVE_PCRE
-#define OVSDB_BASE_STRING_INIT  { .type = OVSDB_TYPE_STRING,        \
-                                  .u.string = { NULL, NULL, NULL,   \
-                                                0, UINT_MAX } }
-#else
 #define OVSDB_BASE_STRING_INIT  { .type = OVSDB_TYPE_STRING,    \
-                                  .u.string = { NULL, NULL,     \
-                                                0, UINT_MAX } }
-#endif
+                                  .u.string = { 0, UINT_MAX } }
 #define OVSDB_BASE_UUID_INIT    { .type = OVSDB_TYPE_UUID,      \
                                   .u.uuid = { NULL, NULL } }
 
@@ -106,10 +90,6 @@ void ovsdb_base_type_destroy(struct ovsdb_base_type *);
 bool ovsdb_base_type_is_valid(const struct ovsdb_base_type *);
 bool ovsdb_base_type_has_constraints(const struct ovsdb_base_type *);
 void ovsdb_base_type_clear_constraints(struct ovsdb_base_type *);
-struct ovsdb_error *ovsdb_base_type_set_regex(struct ovsdb_base_type *,
-                                              const char *reMatch,
-                                              const char *reComment)
-    WARN_UNUSED_RESULT;
 
 struct ovsdb_error *ovsdb_base_type_from_json(struct ovsdb_base_type *,
                                               const struct json *)
index 4020241..adc6dd2 100644 (file)
@@ -177,8 +177,6 @@ is represented by <database-schema>, as described below.
         "maxInteger": <integer>            optional, integers only
         "minReal": <real>                  optional, reals only
         "maxReal": <real>                  optional, reals only 
-        "reMatch": <string>                optional, strings only
-        "reComment": <string>              optional, strings only
         "minLength": <integer>             optional, strings only
         "maxLength": <integer>             optional, strings only
         "refTable": <id>                   optional, uuids only
@@ -196,23 +194,11 @@ is represented by <database-schema>, as described below.
     specified, then the maxReal must be greater than or equal to
     minReal.
 
-    If "type" is "string", then:
-
-        "reMatch" may be a JavaScript (Perl 5-like) regular expression
-        that restricts the allowed values.  The regular expression
-        must match the entire string value, that is, it is treated as
-        if it begins with ^ and ends with $, regardless of whether it
-        really does.
-
-        If "reMatch" is specified, then "reComment" may be a string
-        that describes the allowed values, phrased so that it fits
-        into a sentence such as "This value must be...".
-
-        "minLength" and "maxLength" or both may be specified,
-        restricting the valid length of value strings.  If both are
-        specified, then maxLength must be greater than or equal to
-        minLength.  String length is measured in characters (not bytes
-        or UTF-16 code units).
+    If "type" is "string", then "minLength" and "maxLength" or both
+    may be specified, restricting the valid length of value strings.
+    If both are specified, then maxLength must be greater than or
+    equal to minLength.  String length is measured in characters (not
+    bytes or UTF-16 code units).
 
     If "type" is "uuid", then "refTable", if present, must be the name
     of a table within this database.  If "refTable" is set, the
index 9a90679..c314d61 100755 (executable)
@@ -109,8 +109,10 @@ def escapeCString(src):
     return dst
 
 class BaseType:
-    def __init__(self, type, refTable=None, minInteger=None, maxInteger=None,
-                 minReal=None, maxReal=None, reMatch=None, reComment=None,
+    def __init__(self, type,
+                 refTable=None,
+                 minInteger=None, maxInteger=None,
+                 minReal=None, maxReal=None,
                  minLength=None, maxLength=None):
         self.type = type
         self.refTable = refTable
@@ -118,8 +120,6 @@ class BaseType:
         self.maxInteger = maxInteger
         self.minReal = minReal
         self.maxReal = maxReal
-        self.reMatch = reMatch
-        self.reComment = reComment
         self.minLength = minLength
         self.maxLength = maxLength
 
@@ -134,11 +134,9 @@ class BaseType:
             maxInteger = getMember(json, 'maxInteger', [int, long], description)
             minReal = getMember(json, 'minReal', [int, long, float], description)
             maxReal = getMember(json, 'maxReal', [int, long, float], description)
-            reMatch = getMember(json, 'reMatch', [unicode], description)
-            reComment = getMember(json, 'reComment', [unicode], description)
             minLength = getMember(json, 'minLength', [int], description)
             maxLength = getMember(json, 'minLength', [int], description)
-            return BaseType(atomicType, refTable, minInteger, maxInteger, minReal, maxReal, reMatch, reComment, minLength, maxLength)
+            return BaseType(atomicType, refTable, minInteger, maxInteger, minReal, maxReal, minLength, maxLength)
 
     def toEnglish(self):
         if self.type == 'uuid' and self.refTable:
@@ -192,13 +190,6 @@ class BaseType:
             if self.maxReal != None:
                 stmts.append('%s.u.real.max = %d;' % (var, self.maxReal))
         elif self.type == 'string':
-            if self.reMatch != None:
-                if self.reComment != None:
-                    reComment = '"%s"' % escapeCString(self.reComment)
-                else:
-                    reComment = NULL
-                stmts.append('do_set_regex(&%s, "%s", %s);' % (
-                        var, escapeCString(self.reMatch), reComment))
             if self.minLength != None:
                 stmts.append('%s.u.string.minLen = %d;' % (var, self.minLength))            
             if self.maxLength != None:
@@ -443,21 +434,7 @@ def printCIDLSource(schemaFile):
 #include "ovsdb-error.h"
 
 static bool inited;
-
-static void OVS_UNUSED
-do_set_regex(struct ovsdb_base_type *base, const char *reMatch,
-             const char *reComment)
-{
-    struct ovsdb_error *error;
-
-    error = ovsdb_base_type_set_regex(base, reMatch, reComment);
-    if (error) {
-        char *s = ovsdb_error_to_string(error);
-        ovs_error(0, "%%s", s);
-        free(s);
-        ovsdb_error_destroy(error);
-    }
-}''' % schema.idlHeader
+''' % schema.idlHeader
 
     # Cast functions.
     for tableName, table in sorted(schema.tables.iteritems()):
index ee2905b..07244a8 100644 (file)
@@ -590,14 +590,6 @@ AT_CHECK([RUN_OVS_VSCTL([set b br0 flood_vlans=-1])],
 AT_CHECK([RUN_OVS_VSCTL([set b br0 flood_vlans=4096])], 
   [1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid range 0 to 4095 (inclusive)
 ], [OVS_VSCTL_CLEANUP])
-if test "$HAVE_PCRE" = yes; then
-    AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])], 
-      [1], [], [ovs-vsctl: constraint violation: "xyz" is not a either "in-band" or "out-of-band"
-], [OVS_VSCTL_CLEANUP])
-else
-    AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])], 
-      [0], [], [], [OVS_VSCTL_CLEANUP])
-fi
 AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])], 
   [1], [], [ovs-vsctl: cannot specify key to set for non-map column connection_mode
 ], [OVS_VSCTL_CLEANUP])
index 5ca23cf..031eee0 100644 (file)
@@ -290,37 +290,6 @@ constraint violation: -11 is not in the valid range -10 to 10 (inclusive)
 constraint violation: 11 is not in the valid range -10 to 10 (inclusive)
 constraint violation: 123576 is not in the valid range -10 to 10 (inclusive)])
 
-AT_SETUP([strings matching /(a(b)?)c?/])
-AT_KEYWORDS([ovsdb positive])
-if test "$HAVE_PCRE" = yes; then
-   b_out='constraint violation: "b" does not match regular expression /(a(b)?)?c?/'
-   bc_out='constraint violation: "bc" does not match regular expression /(a(b)?)?c?/'
-else
-   b_out='"b"'
-   bc_out='"bc"'
-fi
-AT_CHECK_UNQUOTED(
-  [[test-ovsdb parse-atoms '{"type": "string", "reMatch": "(a(b)?)?c?"}' \
-    '[""]' \
-    '["a"]' \
-    '["ab"]' \
-    '["abc"]' \
-    '["ac"]' \
-    '["b"]' \
-    '["bc"]' \
-    '["c"]']],
-  [0],
-  [[""
-"a"
-"ab"
-"abc"
-"ac"
-$b_out
-$bc_out
-"c"
-]])
-AT_CLEANUP
-
 OVSDB_CHECK_POSITIVE([strings at least 2 characters long],
   [[parse-atoms '{"type": "string", "minLength": 2}' \
     '[""]' \
index d052776..2d98183 100644 (file)
@@ -44,31 +44,6 @@ OVSDB_CHECK_NEGATIVE([real max may not be less than min],
 OVSDB_CHECK_POSITIVE([boolean], 
   [[parse-base-type '[{"type": "boolean"}]' ]], ["boolean"])
 
-OVSDB_CHECK_POSITIVE([string reMatch], 
-  [[parse-base-type '{"type": "string", "reMatch": "\\d{3}-\\d{3}-\\d{4}"}']],
-  [{"reMatch":"\\d{3}-\\d{3}-\\d{4}","type":"string"}])
-OVSDB_CHECK_POSITIVE([string reMatch + reComment], 
-  [[parse-base-type '{"type": "string", "reMatch": "\\d{3}-\\d{3}-\\d{4}", "reComment": "US-style telephone number"}']],
-  [{"reComment":"US-style telephone number","reMatch":"\\d{3}-\\d{3}-\\d{4}","type":"string"}])
-
-AT_SETUP([reMatch must be a valid regexp])
-AT_KEYWORDS([ovsdb negative])
-if test "$HAVE_PCRE" = yes; then
-   AT_CHECK(
-     [[test-ovsdb parse-base-type \
-                  '{"type": "string", "reMatch": "x{2,1}"}']],
-     [1], [],
-     [[test-ovsdb: invalid regular expression: "x{2,1}" is not a valid regular expression: numbers out of order in {} quantifier
-]])
-else
-   AT_CHECK(
-     [[test-ovsdb parse-base-type \
-                  '{"type": "string", "reMatch": "x{2,1}"}']],
-     [0], [[{"reMatch":"x{2,1}","type":"string"}
-]], [])
-fi
-AT_CLEANUP
-
 OVSDB_CHECK_POSITIVE([string minLength], 
   [[parse-base-type '{"type": "string", "minLength": 1}']],
   [{"minLength":1,"type":"string"}])
index 02fd94e..b75c100 100644 (file)
                   "min": 0, "max": 1}},
        "mac": {
          "comment": "The MAC address to use for this port for the purpose of choosing the bridge's MAC address.  This column does not necessarily reflect the port's actual MAC address, nor will setting it change the port's actual MAC address.",
-         "type": {"key": {"type": "string",
-                          "reMatch": "[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}",
-                          "reComment": "an Ethernet address in the form XX:XX:XX:XX:XX:XX, where each X is a hex digit"},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "bond_updelay": {
          "comment": "For a bonded port, the number of milliseconds for which carrier must stay up on an interface before the interface is considered to be up.  Ignored for non-bonded ports.",
                           "minInteger": 0}}},
        "mac": {
          "comment": "Ethernet address to set for this interface.  If unset then the default MAC address is used.  May not be supported on all interfaces.",
-         "type": {"key": {"type": "string",
-                          "reMatch": "[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}",
-                          "reComment": "an Ethernet address in the form XX:XX:XX:XX:XX:XX, where each X is a hex digit"},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "external_ids": {
          "comment": "Key-value pairs that identify this interface's role in external systems.  The currently defined key-value pairs are: \"xs-vif-uuid\", the UUID of the Citrix XenServer VIF associated with this interface; \"xs-network-uuid\", the UUID of the Citrix XenServer network to which this interface is attached; \"xs-vif-vm-uuid\", the UUID of the Citrix XenServer VM to which this interface belongs; \"xs-vif-mac\", the value of the \"MAC\" field in the Citrix XenServer VIF record for this interface.",
      "columns": {
        "targets": {
          "comment": "NetFlow targets in the form \"IP:PORT\".",
-         "type": {"key": {"type": "string",
-                          "reMatch": "([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])(\\.[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]){3}:[0-9]{1,5}",
-                          "reComment": "an IP address followed by a UDP port number, separated by \":\""},
+         "type": {"key": {"type": "string"},
                   "min": 1, "max": "unlimited"}},
        "engine_type": {
          "comment": "Engine type to use in NetFlow messages.  Defaults to datapath index if not specified.",
          "type": {"key": "integer", "min": 0, "max": 1}},
        "fail_mode": {
          "comment": "Either \"standalone\" or \"secure\", or empty to use the implementation's default.",
-         "type": {"key": {"type": "string",
-                          "reMatch": "standalone|secure",
-                          "reComment": "either \"standalone\" or \"secure\""},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "discover_accept_regex": {
          "comment": "If \"target\" is \"discover\", a POSIX extended regular expression against which the discovered controller location is validated.  If not specified, the default is implementation-specific.",
          "type": {"key": "boolean", "min": 0, "max": 1}},
        "connection_mode": {
          "comment": "Either \"in-band\" or \"out-of-band\".  If not specified, the default is implementation-specific.",
-         "type": {"key": {"type": "string",
-                          "reMatch": "in-band|out-of-band",
-                          "reComment": "either \"in-band\" or \"out-of-band\""},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "local_ip": {
          "comment": "If \"target\" is not \"discover\", the IP address to configure on the local port.",
-         "type": {"key": {"type": "string",
-                          "reMatch": "([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])(\\.[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]){3}",
-                          "reComment": "an IP address"},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "local_netmask": {
          "comment": "If \"target\" is not \"discover\", the IP netmask to configure on the local port.",
-         "type": {"key": {"type": "string", 
-                          "reMatch": "255\\.255\\.255\\.(255|254|252|248|240|224|192|128|0)|255\\.255\\.(255|254|252|248|240|224|192|128|0)\\.0|255\\.(255|254|252|248|240|224|192|128|0)\\.0\\.0|(255|254|252|248|240|224|192|128|0)\\.0\\.0\\.0",
-                          "reComment": "a netmask"},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "local_gateway": {
          "comment": "If \"target\" is not \"discover\", the IP gateway to configure on the local port.",
-         "type": {"key": {"type": "string",
-                          "reMatch": "([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])(\\.[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]){3}",
-                          "reComment": "an IP address"},
+         "type": {"key": {"type": "string"},
                   "min": 0, "max": 1}},
        "controller_rate_limit": {
          "comment": "The maximum rate at which packets will be forwarded to the OpenFlow controller, in packets per second.  If not specified, the default is implementation-specific.",