From 5ef35401510325242737f05af78bc08eb631e8e4 Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Thu, 19 Jun 2014 16:21:31 +0200 Subject: [PATCH] store_lookup: Made the attribute matcher a "standalone" matcher. The attribute matcher will now be applied to host objects directly and then match against that host's attributes. This further simplifies parse_cmp() and the general workflow for matchers. --- src/core/store-private.h | 3 +- src/core/store_lookup.c | 127 ++++++++++++-------------------- src/include/core/store.h | 11 ++- t/unit/core/store_lookup_test.c | 80 ++++++++------------ 4 files changed, 85 insertions(+), 136 deletions(-) diff --git a/src/core/store-private.h b/src/core/store-private.h index 64a70c3..05a9638 100644 --- a/src/core/store-private.h +++ b/src/core/store-private.h @@ -138,7 +138,8 @@ typedef struct { /* match attributes */ typedef struct { - obj_matcher_t super; + sdb_store_matcher_t super; + char *name; /* XXX: this needs to be more flexible; * add support for type-specific operators */ name_matcher_t value; diff --git a/src/core/store_lookup.c b/src/core/store_lookup.c index 1fe45df..fe98e91 100644 --- a/src/core/store_lookup.c +++ b/src/core/store_lookup.c @@ -79,8 +79,6 @@ static int match_logical(sdb_store_matcher_t *m, sdb_store_base_t *obj); static int match_unary(sdb_store_matcher_t *m, sdb_store_base_t *obj); -static int -match_obj(sdb_store_matcher_t *m, sdb_store_base_t *obj); /* specific matchers */ @@ -162,7 +160,7 @@ name_tostring(sdb_store_matcher_t *m, char *buf, size_t buflen) static char * attr_tostring(sdb_store_matcher_t *m, char *buf, size_t buflen) { - char name[buflen + 1], value[buflen + 1]; + char value[buflen + 1]; if (! m) { snprintf(buf, buflen, "ATTR{}"); @@ -170,8 +168,7 @@ attr_tostring(sdb_store_matcher_t *m, char *buf, size_t buflen) } assert(m->type == MATCHER_ATTR); - snprintf(buf, buflen, "ATTR{ NAME%s, VALUE%s }", - obj_name_tostring(&OBJ_M(m)->name, name, sizeof(name)), + snprintf(buf, buflen, "ATTR[%s]{ VALUE%s }", ATTR_M(m)->name, obj_name_tostring(&ATTR_M(m)->value, value, sizeof(value))); return buf; } /* attr_tostring */ @@ -201,6 +198,7 @@ match_name(sdb_store_matcher_t *m, sdb_store_base_t *obj) int status = 0; assert(m && obj); + assert(m->type == MATCHER_NAME); if (obj->type != SDB_HOST) return 0; @@ -228,53 +226,55 @@ match_name(sdb_store_matcher_t *m, sdb_store_base_t *obj) return status; } /* match_name */ -/* match attribute specific values; - * always call this function through match_obj() */ static int -match_attr(attr_matcher_t *m, sdb_store_base_t *obj) +match_attr(sdb_store_matcher_t *m, sdb_store_base_t *obj) { + sdb_llist_iter_t *iter = NULL; + int status = 0; + assert(m && obj); + assert(m->type == MATCHER_ATTR); - if (obj->type != SDB_ATTRIBUTE) + if (obj->type != SDB_HOST) return 0; - { - sdb_attribute_t *attr = SDB_ATTR(obj); + iter = sdb_llist_get_iter(SDB_STORE_OBJ(obj)->attributes); + while (sdb_llist_iter_has_next(iter)) { + sdb_attribute_t *attr = SDB_ATTR(sdb_llist_iter_get_next(iter)); char buf[sdb_data_strlen(&attr->value) + 1]; if (sdb_data_format(&attr->value, buf, sizeof(buf), SDB_UNQUOTED) <= 0) return 0; - return match_obj_name(&m->value, buf); + if (match_obj_name(&ATTR_M(m)->value, buf)) { + status = 1; + break; + } } + sdb_llist_iter_destroy(iter); + return status; } /* match_attr */ /* match host specific values; * always call this function through match_obj() */ static int -match_host(host_matcher_t *m, sdb_store_base_t *obj) +match_host(sdb_store_matcher_t *m, sdb_store_base_t *obj) { - sdb_llist_iter_t *iter; + int status; assert(m && obj); + assert(m->type == MATCHER_HOST); if (obj->type != SDB_HOST) return 0; - if (! m->attr) - return 1; + status = match_obj_name(&OBJ_M(m)->name, obj->super.name); + if (! status) + return status; - iter = sdb_llist_get_iter(SDB_STORE_OBJ(obj)->attributes); - while (sdb_llist_iter_has_next(iter)) { - sdb_store_base_t *attr = STORE_BASE(sdb_llist_iter_get_next(iter)); + if (! HOST_M(m)->attr) + return 1; - /* if any attribute matches, we found a matching host */ - if (match_obj(M(m->attr), attr)) { - sdb_llist_iter_destroy(iter); - return 1; - } - } - sdb_llist_iter_destroy(iter); - return 0; + return match_attr(M(HOST_M(m)->attr), obj); } /* match_host */ /* generic matchers */ @@ -288,8 +288,8 @@ static matcher_cb matchers[] = { match_logical, match_unary, match_name, - match_obj, - match_obj, + match_attr, + match_host, }; typedef char *(*matcher_tostring_cb)(sdb_store_matcher_t *, char *, size_t); @@ -330,31 +330,6 @@ match_unary(sdb_store_matcher_t *m, sdb_store_base_t *obj) return !sdb_store_matcher_matches(UOP_M(m)->op, obj); } /* match_unary */ -static int -match_obj(sdb_store_matcher_t *m, sdb_store_base_t *obj) -{ - int status; - - assert(m && obj); - - status = match_obj_name(&OBJ_M(m)->name, obj->super.name); - if (! status) - return status; - - switch (m->type) { - case MATCHER_ATTR: - return match_attr(ATTR_M(m), obj); - break; - case MATCHER_HOST: - return match_host(HOST_M(m), obj); - break; - default: - assert(m->type != m->type); - break; - } - return 0; -} /* match_obj */ - /* * private matcher types */ @@ -412,22 +387,24 @@ static int attr_matcher_init(sdb_object_t *obj, va_list ap) { attr_matcher_t *attr = ATTR_M(obj); - int status; - - status = obj_matcher_init(obj, ap); - if (! status) - status = name_matcher_init(&attr->value, ap); + const char *name = va_arg(ap, const char *); M(obj)->type = MATCHER_ATTR; - return status; + if (name) { + attr->name = strdup(name); + if (! attr->name) + return -1; + } + return name_matcher_init(&attr->value, ap); } /* attr_matcher_init */ static void attr_matcher_destroy(sdb_object_t *obj) { attr_matcher_t *attr = ATTR_M(obj); - - obj_matcher_destroy(obj); + if (attr->name) + free(attr->name); + attr->name = NULL; name_matcher_destroy(&attr->value); } /* attr_matcher_destroy */ @@ -558,11 +535,13 @@ sdb_store_name_matcher(int type, const char *name, _Bool re) } /* sdb_store_name_matcher */ sdb_store_matcher_t * -sdb_store_attr_matcher(const char *attr_name, const char *attr_name_re, - const char *attr_value, const char *attr_value_re) +sdb_store_attr_matcher(const char *name, const char *value, _Bool re) { + if (re) + return M(sdb_object_create("attr-matcher", attr_type, + name, NULL, value)); return M(sdb_object_create("attr-matcher", attr_type, - attr_name, attr_name_re, attr_value, attr_value_re)); + name, value, NULL)); } /* sdb_store_attr_matcher */ sdb_store_matcher_t * @@ -583,9 +562,6 @@ sdb_store_matcher_parse_cmp(const char *obj_type, const char *attr, sdb_store_matcher_t *m = NULL; - const char *matcher = NULL; - const char *matcher_re = NULL; - if (! strcasecmp(obj_type, "host")) type = SDB_HOST; else if (! strcasecmp(obj_type, "service")) @@ -595,18 +571,15 @@ sdb_store_matcher_parse_cmp(const char *obj_type, const char *attr, /* TODO: support other operators */ if (! strcasecmp(op, "=")) { - matcher = value; + /* nothing to do */ } else if (! strcasecmp(op, "!=")) { - matcher = value; inv = 1; } else if (! strcasecmp(op, "=~")) { - matcher_re = value; re = 1; } else if (! strcasecmp(op, "!~")) { - matcher_re = value; inv = 1; re = 1; } @@ -615,14 +588,8 @@ sdb_store_matcher_parse_cmp(const char *obj_type, const char *attr, if (! strcasecmp(attr, "name")) m = sdb_store_name_matcher(type, value, re); - else if (type == SDB_ATTRIBUTE) { - m = sdb_store_host_matcher(/* name = */ NULL, NULL, - sdb_store_attr_matcher(attr, NULL, matcher, matcher_re)); - - /* pass ownership to the host matcher */ - if (m) - sdb_object_deref(SDB_OBJ(HOST_M(m)->attr)); - } + else if (type == SDB_ATTRIBUTE) + m = sdb_store_attr_matcher(attr, value, re); if (! m) return NULL; diff --git a/src/include/core/store.h b/src/include/core/store.h index 856e19c..e4dcd57 100644 --- a/src/include/core/store.h +++ b/src/include/core/store.h @@ -160,20 +160,19 @@ typedef struct sdb_store_matcher sdb_store_matcher_t; * sdb_store_name_matcher: * Creates a matcher matching by the specified object type's name. If 're' is * true, the specified name is treated as a POSIX extended regular expression. - * Else, the exact name has to match. + * Else, the exact name has to match (case-insensitive). */ sdb_store_matcher_t * sdb_store_name_matcher(int type, const char *name, _Bool re); /* * sdb_store_attr_matcher: - * Creates a matcher matching attributes based on their name or value. Either - * a complete name (which will have to match completely but case-independent) - * or an extended POSIX regular expression may be specified. + * Creates a matcher matching attributes based on their value. If 're' is + * true, the specified name is treated as a POSIX extended regular expression. + * Else, the exact name has to match (case-insensitive). */ sdb_store_matcher_t * -sdb_store_attr_matcher(const char *attr_name, const char *attr_name_re, - const char *attr_value, const char *attr_value_re); +sdb_store_attr_matcher(const char *name, const char *value, _Bool re); /* * sdb_store_host_matcher: diff --git a/t/unit/core/store_lookup_test.c b/t/unit/core/store_lookup_test.c index 1a55d87..4bbfcc2 100644 --- a/t/unit/core/store_lookup_test.c +++ b/t/unit/core/store_lookup_test.c @@ -91,63 +91,58 @@ START_TEST(test_store_match) const char *hostname_re; const char *attr_name; - const char *attr_name_re; const char *attr_value; - const char *attr_value_re; + _Bool re; int expected; } golden_data[] = { { /* host */ NULL, NULL, - /* attr */ NULL, NULL, NULL, NULL, 1 + /* attr */ NULL, NULL, 0, 1 + }, + { + /* host */ NULL, NULL, + /* attr */ NULL, NULL, 1, 1 }, { /* host */ "a", NULL, - /* attr */ NULL, NULL, NULL, NULL, 1 + /* attr */ NULL, NULL, 0, 1 }, { /* host */ "b", NULL, - /* attr */ NULL, NULL, NULL, NULL, 0 + /* attr */ NULL, NULL, 0, 0 }, { /* host */ NULL, "^a$", - /* attr */ NULL, NULL, NULL, NULL, 1 + /* attr */ NULL, NULL, 0, 1 }, { /* host */ NULL, "^b$", - /* attr */ NULL, NULL, NULL, NULL, 0 + /* attr */ NULL, NULL, 0, 0 }, { /* host */ "a", "^a$", - /* attr */ NULL, NULL, NULL, NULL, 1 + /* attr */ NULL, NULL, 0, 1 }, { /* host */ "a", "^b$", - /* attr */ NULL, NULL, NULL, NULL, 0 + /* attr */ NULL, NULL, 0, 0 }, { /* host */ "b", "^a$", - /* attr */ NULL, NULL, NULL, NULL, 0 - }, - { - /* host */ "a", "^a$", - /* attr */ "k1", NULL, NULL, NULL, 1 + /* attr */ NULL, NULL, 0, 0 }, { /* host */ "a", "^a$", - /* attr */ NULL, "^k", NULL, NULL, 1 + /* attr */ "k1", NULL, 0, 1 }, { /* host */ "a", "^a$", - /* attr */ NULL, NULL, "v1", NULL, 1 + /* attr */ NULL, "v1", 0, 1 }, { /* host */ "a", "^a$", - /* attr */ NULL, NULL, NULL, "^v1$", 1 - }, - { - /* host */ "a", "^a$", - /* attr */ "k1", "1", "v1", "1", 1 + /* attr */ NULL, "^v1$", 1, 1 }, }; @@ -159,11 +154,11 @@ START_TEST(test_store_match) for (i = 0; i < SDB_STATIC_ARRAY_LEN(golden_data); ++i) { sdb_store_matcher_t *h, *a, *n; + char buf[1024]; int status; a = sdb_store_attr_matcher(golden_data[i].attr_name, - golden_data[i].attr_name_re, golden_data[i].attr_value, - golden_data[i].attr_value_re); + golden_data[i].attr_value, golden_data[i].re); fail_unless(a != NULL, "sdb_store_attr_matcher() = NULL; expected: "); @@ -176,11 +171,8 @@ START_TEST(test_store_match) status = sdb_store_matcher_matches(h, obj); fail_unless(status == golden_data[i].expected, - "sdb_store_matcher_matches({{%s, %s}," - "{%s, %s, %s, %s}}, ) = %d; expected: %d", - golden_data[i].hostname, golden_data[i].hostname_re, - golden_data[i].attr_name, golden_data[i].attr_name_re, - golden_data[i].attr_value, golden_data[i].attr_value_re, + "sdb_store_matcher_matches({%s, ) = %d; expected: %d", + sdb_store_matcher_tostring(h, buf, sizeof(buf)), status, golden_data[i].expected); n = sdb_store_inv_matcher(h); @@ -191,11 +183,8 @@ START_TEST(test_store_match) /* now match the inverted set of objects */ status = sdb_store_matcher_matches(n, obj); fail_unless(status == !golden_data[i].expected, - "sdb_store_matcher_matches(NOT{{%s, %s}," - "{%s, %s, %s, %s}}, ) = %d; expected: %d", - golden_data[i].hostname, golden_data[i].hostname_re, - golden_data[i].attr_name, golden_data[i].attr_name_re, - golden_data[i].attr_value, golden_data[i].attr_value_re, + "sdb_store_matcher_matches({%s, ) = %d; expected: %d", + sdb_store_matcher_tostring(n, buf, sizeof(buf)), status, !golden_data[i].expected); sdb_object_deref(SDB_OBJ(n)); @@ -280,7 +269,7 @@ START_TEST(test_store_match_name) fail_unless(status == !golden_data[i].expected, "sdb_store_matcher_matches(%s, ) = %d; expected: %d", sdb_store_matcher_tostring(n, buf, sizeof(buf)), - status, golden_data[i].expected); + status, !golden_data[i].expected); sdb_object_deref(SDB_OBJ(n)); } @@ -390,9 +379,9 @@ START_TEST(test_parse_cmp) { "attribute", "name", "!=", "attrname", MATCHER_NOT }, { "attribute", "name", "=~", "attrname", MATCHER_NAME }, { "attribute", "name", "!~", "attrname", MATCHER_NOT }, - { "attribute", "attr", "=", "attrname", MATCHER_HOST }, + { "attribute", "attr", "=", "attrname", MATCHER_ATTR }, { "attribute", "attr", "!=", "attrname", MATCHER_NOT }, - { "attribute", "attr", "=~", "attrname", MATCHER_HOST }, + { "attribute", "attr", "=~", "attrname", MATCHER_ATTR }, { "attribute", "attr", "!~", "attrname", MATCHER_NOT }, { "attribute", "attr", "&^", "attrname", -1 }, }; @@ -469,23 +458,16 @@ START_TEST(test_lookup) { "attribute.name = 'x'", 0, "OBJ\\[attribute\\]\\{ NAME\\{ 'x', \\(nil\\) \\}" }, { "attribute.k1 = 'v1'", 1, - "HOST\\{ NAME\\{ NULL, \\(nil\\) \\}, ATTR\\{ " - "NAME\\{ 'k1', \\(nil\\) }, VALUE\\{ 'v1', \\(nil\\) \\} " - "\\} \\}" }, + "ATTR\\[k1\\]\\{ VALUE\\{ 'v1', \\(nil\\) \\} \\}" }, { "attribute.k1 != 'v1'", 2, - "\\(NOT, HOST\\{ NAME\\{ NULL, \\(nil\\) \\}, ATTR\\{ " - "NAME\\{ 'k1', \\(nil\\) }, VALUE\\{ 'v1', \\(nil\\) \\} " - "\\} \\})" }, + "\\(NOT, ATTR\\[k1\\]\\{ VALUE\\{ 'v1', \\(nil\\) \\} \\}\\)" }, { "attribute.k1 != 'v2'", 3, - "\\(NOT, HOST\\{ NAME\\{ NULL, \\(nil\\) \\}, ATTR\\{ " - "NAME\\{ 'k1', \\(nil\\) }, VALUE\\{ 'v2', \\(nil\\) \\} " - "\\} \\})" }, + "\\(NOT, ATTR\\[k1\\]\\{ VALUE\\{ 'v2', \\(nil\\) \\} \\}\\)" }, { "attribute.name != 'x' " "AND attribute.y !~ 'x'", 3, - "\\(AND, \\(NOT, OBJ\\[attribute\\]\\{ NAME\\{ 'x', \\(nil\\) \\} " - "\\}\\), \\(NOT, HOST\\{ NAME\\{ NULL, \\(nil\\) \\}, ATTR\\{ " - "NAME\\{ 'y', \\(nil\\) }, VALUE\\{ NULL, "PTR_RE" \\} " - "\\} \\}\\)\\)" }, + "\\(AND, " + "\\(NOT, OBJ\\[attribute\\]\\{ NAME\\{ 'x', \\(nil\\) \\} \\}\\), " + "\\(NOT, ATTR\\[y\\]\\{ VALUE\\{ NULL, "PTR_RE" \\} \\}\\)\\)" }, }; int check, n; -- 2.30.2