m-peko / booleval Goto Github PK

Benchmark results are necessary in order not to worsen performance by every following change.

This code is awesome and exactly what I was looking for, thanks for developing and sharing @m-peko

The one thing that is missing, or I can't seem to get to work, is being able to quote string constants.
i.e. field_name == 'string with spaces'

Is there a way to do this currently or is this feature planned?

Improve performance of building expression tree.
Now, building expression tree takes too long, which is apparent from the unit tests.

Mark expressions like src ip 192.168.9.43, with 3 fields (i.e. 3 non-keywords) in a row, as invalid.
More generally, improve checking of string expression.

Test code coverage in Travis CI.

It would be nice to have a visitor derived class used for printing expression tree in the following form (field_a foo and field_b bar).

Is it possible to implement set operations (intersection/etc) for the current implementation? If so, how would they look?
I suppose each set will be represented as a balanced subtree? Thx.

Hi Marin,

I have some evaluator fields which can sometimes not be available (be null) and it would be good if there was a clean way to handle this. I have been experimenting with ways to do this and the easiest solution I have come up with is to allow the evaluator field_map functions to throw a specialised booleval exception (based on std::exception or std::runtime_error) i.e. booleval_field_null. We could catch this new booleval_field_null exception in result_visitor.visit_relational() and then rethrow with more context (i.e. name of the field) and then it can be caught by the code that calls evaluator.evaluate() (outside of booleval code) and handled/logged there.

There is also an unhandled exception in result_visitor.visit_relational if key.value() is not found in the fields_ map which could also throw an booleval_field_unknown exception.

Do you have any thoughts on this?

Cheers

James

Given:

obj pass("21", 123);

where field_a is of type std::string,

evaluator.expression("field_a < "121"");

Evaluates to true which is incorrect when comparing strings. It should evaluate to false as for string comparison, "21" is greater than "123" whereas when evaluating as a number 21 is less than 123.

In this example, field_a is of type std::string and the comparison is with "123" (quoted string), so string comparison should be used and not arithmetic comparison.

Hi Marin,

There are a few classes that are missing class initializers. I will submit a pull request for this shortly.

Cheers

James

I just started to integrate booleval and found this issue where the object I'm evaluating is being deleted multiple times by booleval, resulting in a core dump because in my case its triggering a double free.

I have reproduced the behavior in a small snippet (ignore the malloc/free ... that just mirrors my actual use case and is what drew my attention to the issues when i core dumped for a double free)

#include <stdio.h>
#include <string_view>
#include <booleval/evaluator.hpp>

struct MyClass {
    size_t _seq;
    void * buffer;

    MyClass(size_t seq) {
        printf(">> MyClass::ctor\n");

        //buffer = malloc(1000);
        _seq = seq;
    }
    
    ~MyClass() {
        printf("<< MyClass::dtor\n");
        //free(buffer);
    }
    
    size_t seq() { 
        return _seq; 
    }
};


u_int32_t count(std::string_view filter) {

   u_int32_t count=0;

    booleval::evaluator evaluator({
        {"seq", &MyClass::seq }
    });

    auto valid = evaluator.expression(filter);
    if (!valid) {
       throw std::runtime_error("count_pdus expression not valid");
    }

    std::vector<MyClass*> vector;

    for(int x=0;x<10;x++) {
        MyClass* c = new MyClass(x);
        vector.push_back(c);
    }

    for(MyClass* cc : vector) {

        if (evaluator.is_activated()) {
            if(evaluator.evaluate(*cc)) ++count;
        } else {
            throw std::runtime_error("Evaluator is not activated");
        }
   }
  return count;
}

int main(void) {
  u_int32_t x = count("(seq == 1)");
  printf("count = %u\n",x);
  return 0;
}

If you were to run that you would see multiple calls to the destructor for each object evaluated. If uncomment the malloc/free gdb shows this backtrace:

#0  0x00007ffff6e03387 in raise () from /lib64/libc.so.6
#1  0x00007ffff6e04a78 in abort () from /lib64/libc.so.6
#2  0x00007ffff6e45ed7 in __libc_message () from /lib64/libc.so.6
#3  0x00007ffff6e4e299 in _int_free () from /lib64/libc.so.6
#4  0x00007ffff7770e23 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() () from /lib64/libstdc++.so.6
#5  0x00000000004ac94e in booleval::utils::any_value::~any_value (this=0x7ffff4e84370, __in_chrg=<optimized out>)
    at /home/rjo/src/iascollector/booleval/include/booleval/utils/any_value.hpp:120
#6  0x00000000004acafe in booleval::tree::result_visitor<booleval::utils::any_mem_fn>::visit_relational<MyClass, std::equal_to<void> >(booleval::tree::tree_node const&, MyClass const&, std::equal_to<void>&&) (this=0x7ffff4e84498, node=..., obj=..., func=<unknown type in /home/rjo/src/iascollector/build/iascollector, CU 0x3e5544, DIE 0x432b1a>)
    at /home/rjo/src/iascollector/booleval/include/booleval/tree/result_visitor.hpp:119
#7  0x00000000004abc17 in booleval::tree::result_visitor<booleval::utils::any_mem_fn>::visit<MyClass> (this=0x7ffff4e84498, node=..., obj=...)
    at /home/rjo/src/iascollector/booleval/include/booleval/tree/result_visitor.hpp:141
#8  0x00000000004ab0e8 in booleval::evaluator<booleval::utils::any_mem_fn>::evaluate<MyClass> (this=0x7ffff4e84490, obj=...)
    at /home/rjo/src/iascollector/booleval/include/booleval/evaluator.hpp:103

I should note that the same behavior occurs if a non-pointer type is used in the evaluation.

Any idea whats going on here?

As per the discussion on fluentcpp, it would be useful to be able to quote a field to allow space(s) and reserved words in the field. eg

field_a "a test" and field_b 123
field_a "and" and field_b 123

If you try:

obja pass("21", 123.1);
objb fail("bar", 456.2);

booleval::evaluator evaluator({
    { "field_aa", &obja::field_aa },
    { "field_bb", &objb::field_bb }
    });

evaluator.expression("field_aa \"21\" and field_bb 123.1");

if (evaluator.is_activated()) {
    std::cout << std::boolalpha << evaluator.evaluate(pass) << std::endl;
    //std::cout << std::boolalpha << evaluator.evaluate(fail) << std::endl;
} else
    std::cerr << "Expression not valid!" << std::endl;

Where you are trying to evaluate an expression with fields defined from multiple classes, then although this compiles OK, you get at run-time un-handled exception error.

Wouldn't it be better if evaluate() returns false if is_activated_ is false? Returning true gives the wrong impression that the expression has been evaluated when it hasn't if is_activated_ is false??

Hi @m-peko.

I have run into an issue which I am struggling to solve at the moment. The symbols >= and <= are not working whereas GEQ and LEQ are working so its something to do with the parsing as it seems to think they are > or < and the expression fails as not valid.

I will continue looking and trying to solve it but wanted you to know there is an issue there.

Cheers

James

As the title is saying, use member function pointers instead of std::map for evaluation. Once the member function pointers are set, function can be called for each object passed to the evaluator by using std::invoke.

Add documentation of classes, function members, etc.

Use C++17 standard along with all the features it provides.

A quoted keyword is considered an invalid expression

obj pass("and", 123);
...
evaluator.expression("field_a "and" and field_b 123");

should be valid as "and" is a literal and not a keyword - but is considered a keyword and rejected as an invalid expression.

Add more functionalities to RegexPattern class, possibly implement state machine as a part of class.

Currently, Evaluator class holds functionality related to parsing expression tree. Better design would be to extract this functionality into a new class ExpressionTree.