From ac66c9430defd2a0e187e7b3679c8ebbfda8a4bb Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Thu, 6 Nov 2014 00:20:20 +0100 Subject: [PATCH] frontend: Let the analyzer report details about errors. --- src/frontend/analyzer.c | 128 +++++++++++++++++++++++----------- src/frontend/parser.c | 5 +- src/frontend/query.c | 9 ++- src/include/frontend/parser.h | 5 +- 4 files changed, 98 insertions(+), 49 deletions(-) diff --git a/src/frontend/analyzer.c b/src/frontend/analyzer.c index 93debf5..5cb1f7f 100644 --- a/src/frontend/analyzer.c +++ b/src/frontend/analyzer.c @@ -27,9 +27,11 @@ #include "sysdb.h" -#include -#include -#include +#include "core/store-private.h" +#include "frontend/connection-private.h" +#include "frontend/parser.h" +#include "utils/error.h" +#include "utils/strbuf.h" #include @@ -37,11 +39,25 @@ * private helper functions */ -static int -analyze_matcher(int context, sdb_store_matcher_t *m) +static void +iter_error(sdb_strbuf_t *errbuf, int op, int oper, int context) { - int status = 0; + sdb_strbuf_sprintf(errbuf, "Cannot use %s %s in %s context", + MATCHER_SYM(op), SDB_STORE_TYPE_TO_NAME(oper), + SDB_STORE_TYPE_TO_NAME(context)); +} /* iter_error */ + +static void +cmp_error(sdb_strbuf_t *errbuf, int op, int left, int right) +{ + sdb_strbuf_sprintf(errbuf, "Invalid operator %s for types %s and %s", + MATCHER_SYM(op), SDB_TYPE_TO_STRING(left), + SDB_TYPE_TO_STRING(right)); +} /* iter_error */ +static int +analyze_matcher(int context, sdb_store_matcher_t *m, sdb_strbuf_t *errbuf) +{ if (! m) return 0; @@ -49,39 +65,49 @@ analyze_matcher(int context, sdb_store_matcher_t *m) case MATCHER_OR: case MATCHER_AND: assert(OP_M(m)->left && OP_M(m)->right); - if (analyze_matcher(context, OP_M(m)->left)) - status = -1; - if (analyze_matcher(context, OP_M(m)->right)) - status = -1; + if (analyze_matcher(context, OP_M(m)->left, errbuf)) + return -1; + if (analyze_matcher(context, OP_M(m)->right, errbuf)) + return -1; break; case MATCHER_NOT: assert(UOP_M(m)->op); - if (analyze_matcher(context, UOP_M(m)->op)) - status = -1; + if (analyze_matcher(context, UOP_M(m)->op, errbuf)) + return -1; break; case MATCHER_ANY: case MATCHER_ALL: assert(ITER_M(m)->m); - if (ITER_M(m)->type == context) - status = -1; if ((context != SDB_HOST) && (context != SDB_SERVICE) - && (context != SDB_METRIC)) - status = -1; + && (context != SDB_METRIC)) { + iter_error(errbuf, m->type, ITER_M(m)->type, context); + return -1; + } + if (ITER_M(m)->type == context) { + iter_error(errbuf, m->type, ITER_M(m)->type, context); + return -1; + } if ((ITER_M(m)->type != SDB_SERVICE) && (ITER_M(m)->type != SDB_METRIC) - && (ITER_M(m)->type != SDB_ATTRIBUTE)) - status = -1; + && (ITER_M(m)->type != SDB_ATTRIBUTE)) { + iter_error(errbuf, m->type, ITER_M(m)->type, context); + return -1; + } if ((context == SDB_SERVICE) - && (ITER_M(m)->type == SDB_METRIC)) - status = -1; + && (ITER_M(m)->type == SDB_METRIC)) { + iter_error(errbuf, m->type, ITER_M(m)->type, context); + return -1; + } else if ((context == SDB_METRIC) - && (ITER_M(m)->type == SDB_SERVICE)) - status = -1; - if (analyze_matcher(ITER_M(m)->type, ITER_M(m)->m)) - status = -1; + && (ITER_M(m)->type == SDB_SERVICE)) { + iter_error(errbuf, m->type, ITER_M(m)->type, context); + return -1; + } + if (analyze_matcher(ITER_M(m)->type, ITER_M(m)->m, errbuf)) + return -1; break; case MATCHER_LT: @@ -92,20 +118,32 @@ analyze_matcher(int context, sdb_store_matcher_t *m) case MATCHER_GT: assert(CMP_M(m)->left && CMP_M(m)->right); if ((CMP_M(m)->left->data_type > 0) - && (CMP_M(m)->left->data_type & SDB_TYPE_ARRAY)) - status = -1; + && (CMP_M(m)->left->data_type & SDB_TYPE_ARRAY)) { + cmp_error(errbuf, m->type, CMP_M(m)->left->data_type, + CMP_M(m)->right->data_type); + return -1; + } if ((CMP_M(m)->right->data_type > 0) - && (CMP_M(m)->right->data_type & SDB_TYPE_ARRAY)) - status = -1; + && (CMP_M(m)->right->data_type & SDB_TYPE_ARRAY)) { + cmp_error(errbuf, m->type, CMP_M(m)->left->data_type, + CMP_M(m)->right->data_type); + return -1; + } break; case MATCHER_IN: if ((CMP_M(m)->left->data_type > 0) - && (CMP_M(m)->left->data_type & SDB_TYPE_ARRAY)) - status = -1; + && (CMP_M(m)->left->data_type & SDB_TYPE_ARRAY)) { + cmp_error(errbuf, m->type, CMP_M(m)->left->data_type, + CMP_M(m)->right->data_type); + return -1; + } if ((CMP_M(m)->right->data_type > 0) - && (! (CMP_M(m)->right->data_type & SDB_TYPE_ARRAY))) - status = -1; + && (! (CMP_M(m)->right->data_type & SDB_TYPE_ARRAY))) { + cmp_error(errbuf, m->type, CMP_M(m)->left->data_type, + CMP_M(m)->right->data_type); + return -1; + } break; case MATCHER_REGEX: @@ -113,8 +151,11 @@ analyze_matcher(int context, sdb_store_matcher_t *m) /* all types are supported for the left operand */ if ((CMP_M(m)->right->data_type > 0) && (CMP_M(m)->right->data_type != SDB_TYPE_REGEX) - && (CMP_M(m)->right->data_type != SDB_TYPE_STRING)) - status = -1; + && (CMP_M(m)->right->data_type != SDB_TYPE_STRING)) { + cmp_error(errbuf, m->type, CMP_M(m)->left->data_type, + CMP_M(m)->right->data_type); + return -1; + } break; case MATCHER_ISNULL: @@ -122,9 +163,10 @@ analyze_matcher(int context, sdb_store_matcher_t *m) break; default: + sdb_strbuf_sprintf(errbuf, "Unknown matcher type %d", m->type); return -1; } - return status; + return 0; } /* analyze_matcher */ /* @@ -132,7 +174,7 @@ analyze_matcher(int context, sdb_store_matcher_t *m) */ int -sdb_fe_analyze(sdb_conn_node_t *node) +sdb_fe_analyze(sdb_conn_node_t *node, sdb_strbuf_t *errbuf) { sdb_store_matcher_t *m = NULL, *filter = NULL; int context = -1; @@ -145,9 +187,15 @@ sdb_fe_analyze(sdb_conn_node_t *node) * later, this may be turned into one of multiple AST visitors. */ if (node->cmd == CONNECTION_FETCH) { conn_fetch_t *fetch = CONN_FETCH(node); - if (((fetch->type == SDB_HOST) && fetch->name) - || ((fetch->type != SDB_HOST) && (! fetch->name))) + if ((fetch->type == SDB_HOST) && fetch->name) { + sdb_strbuf_sprintf(errbuf, "Unexpected STRING '%s'", fetch->name); + return -1; + } + if ((fetch->type != SDB_HOST) && (! fetch->name)) { + sdb_strbuf_sprintf(errbuf, "Missing %s name", + SDB_STORE_TYPE_TO_NAME(fetch->type)); return -1; + } if (fetch->filter) filter = fetch->filter->matcher; context = fetch->type; @@ -169,9 +217,9 @@ sdb_fe_analyze(sdb_conn_node_t *node) else return -1; - if (analyze_matcher(context, m)) + if (analyze_matcher(context, m, errbuf)) status = -1; - if (analyze_matcher(-1, filter)) + if (analyze_matcher(-1, filter, errbuf)) status = -1; return status; } /* sdb_fe_analyze */ diff --git a/src/frontend/parser.c b/src/frontend/parser.c index 936f31e..c38799d 100644 --- a/src/frontend/parser.c +++ b/src/frontend/parser.c @@ -98,10 +98,7 @@ sdb_fe_parse(const char *query, int len, sdb_strbuf_t *errbuf) while (sdb_llist_iter_has_next(iter)) { sdb_conn_node_t *node; node = SDB_CONN_NODE(sdb_llist_iter_get_next(iter)); - if (sdb_fe_analyze(node)) { - /* TODO: pass on errbuf to the analyzer */ - sdb_strbuf_sprintf(errbuf, "Failed to verify " - "query '%s'", query); + if (sdb_fe_analyze(node, errbuf)) { sdb_llist_iter_destroy(iter); sdb_llist_destroy(yyextra.parsetree); return NULL; diff --git a/src/frontend/query.c b/src/frontend/query.c index 338d95a..34b3583 100644 --- a/src/frontend/query.c +++ b/src/frontend/query.c @@ -208,12 +208,15 @@ sdb_fe_lookup(sdb_conn_t *conn) /* run analyzer separately; parse_matcher is missing * the right context to do so */ - if (sdb_fe_analyze(SDB_CONN_NODE(&node))) { + if (sdb_fe_analyze(SDB_CONN_NODE(&node), conn->errbuf)) { char expr[matcher_len + 1]; + char err[sdb_strbuf_len(conn->errbuf) + sizeof(expr) + 64]; strncpy(expr, matcher, sizeof(expr)); expr[sizeof(expr) - 1] = '\0'; - sdb_strbuf_sprintf(conn->errbuf, "Failed to verify " - "lookup condition '%s'", expr); + snprintf(err, sizeof(err), "Failed to parse " + "lookup condition '%s': %s", expr, + sdb_strbuf_string(conn->errbuf)); + sdb_strbuf_sprintf(conn->errbuf, "%s", err); status = -1; } else diff --git a/src/include/frontend/parser.h b/src/include/frontend/parser.h index e763afc..b451ed5 100644 --- a/src/include/frontend/parser.h +++ b/src/include/frontend/parser.h @@ -76,14 +76,15 @@ sdb_fe_parse_expr(const char *expr, int len, sdb_strbuf_t *errbuf); /* * sdb_fe_analyze: - * Analyze a parsed node, checking for semantical errors. + * Analyze a parsed node, checking for semantical errors. Error messages will + * be written to the string buffer, if provided. * * Returns: * - 0 if the node is semantically correct * - a negative value else */ int -sdb_fe_analyze(sdb_conn_node_t *node); +sdb_fe_analyze(sdb_conn_node_t *node, sdb_strbuf_t *errbuf); #ifdef __cplusplus } /* extern "C" */ -- 2.30.2