From 89521e3f79d53523fda9f284e74f0f7aa21332b4 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 23 Feb 2010 15:57:50 -0800 Subject: [PATCH] ovsdb: Drop regular expression constraints. 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 | 24 ---------- lib/ovsdb-types.c | 91 +------------------------------------- lib/ovsdb-types.h | 22 +-------- ovsdb/SPECS | 24 +++------- ovsdb/ovsdb-idlc.in | 35 +++------------ tests/ovs-vsctl.at | 8 ---- tests/ovsdb-data.at | 31 ------------- tests/ovsdb-types.at | 25 ----------- vswitchd/vswitch.ovsschema | 32 ++++---------- 9 files changed, 21 insertions(+), 271 deletions(-) diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c index 08d623a5..9d8eab85 100644 --- a/lib/ovsdb-data.c +++ b/lib/ovsdb-data.c @@ -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; } diff --git a/lib/ovsdb-types.c b/lib/ovsdb-types.c index 5b7e7d8f..5da71456 100644 --- a/lib/ovsdb-types.c +++ b/lib/ovsdb-types.c @@ -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)); diff --git a/lib/ovsdb-types.h b/lib/ovsdb-types.h index 0eca931f..0ef596ab 100644 --- a/lib/ovsdb-types.h +++ b/lib/ovsdb-types.h @@ -22,10 +22,6 @@ #include "compiler.h" #include "uuid.h" -#ifdef HAVE_PCRE -#include -#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 *) diff --git a/ovsdb/SPECS b/ovsdb/SPECS index 4020241e..adc6dd22 100644 --- a/ovsdb/SPECS +++ b/ovsdb/SPECS @@ -177,8 +177,6 @@ is represented by , as described below. "maxInteger": optional, integers only "minReal": optional, reals only "maxReal": optional, reals only - "reMatch": optional, strings only - "reComment": optional, strings only "minLength": optional, strings only "maxLength": optional, strings only "refTable": optional, uuids only @@ -196,23 +194,11 @@ is represented by , 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 diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index 9a90679e..c314d615 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -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()): diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index ee2905b4..07244a82 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -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]) diff --git a/tests/ovsdb-data.at b/tests/ovsdb-data.at index 5ca23cf1..031eee04 100644 --- a/tests/ovsdb-data.at +++ b/tests/ovsdb-data.at @@ -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}' \ '[""]' \ diff --git a/tests/ovsdb-types.at b/tests/ovsdb-types.at index d0527766..2d981833 100644 --- a/tests/ovsdb-types.at +++ b/tests/ovsdb-types.at @@ -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"}]) diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema index 02fd94e2..b75c100c 100644 --- a/vswitchd/vswitch.ovsschema +++ b/vswitchd/vswitch.ovsschema @@ -103,9 +103,7 @@ "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.", @@ -147,9 +145,7 @@ "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.", @@ -194,9 +190,7 @@ "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.", @@ -251,9 +245,7 @@ "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.", @@ -263,27 +255,19 @@ "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.", -- 2.30.2