From 24d825ca512e25b8f38626d3f626107f6a1e6e8b Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 26 Dec 2024 21:29:23 +0100 Subject: [PATCH 01/47] lib/perf: add support for perf based profiling Signed-off-by: Balazs Scheidler --- configure.ac | 1 + lib/Makefile.am | 4 +- lib/apphook.c | 3 + lib/perf/Makefile.am | 8 +++ lib/perf/perf.c | 159 ++++++++++++++++++++++++++++++++++++++++++ lib/perf/perf.h | 38 ++++++++++ lib/perf/trampoline.S | 19 +++++ 7 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 lib/perf/Makefile.am create mode 100644 lib/perf/perf.c create mode 100644 lib/perf/perf.h create mode 100644 lib/perf/trampoline.S diff --git a/configure.ac b/configure.ac index f906a0dfa8..f6e8484be1 100644 --- a/configure.ac +++ b/configure.ac @@ -466,6 +466,7 @@ dnl Checks for programs. AC_PROG_CC AC_PROG_CC_C99 AM_PROG_CC_C_O +AM_PROG_AS if test "x$ac_cv_prog_cc_c99" = "xno"; then AC_MSG_ERROR([C99 standard compliant C compiler required. Try GCC 3.x or later.]) fi diff --git a/lib/Makefile.am b/lib/Makefile.am index 995360761b..b4492b22f7 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -29,6 +29,7 @@ include lib/logthrsource/Makefile.am include lib/logthrdest/Makefile.am include lib/signal-slot-connector/Makefile.am include lib/multi-line/Makefile.am +include lib/perf/Makefile.am LSNG_RELEASE = $(shell echo @PACKAGE_VERSION@ | cut -d. -f1,2) @@ -307,7 +308,8 @@ lib_libsyslog_ng_la_SOURCES = \ $(multiline_sources) \ $(logthrsource_sources) \ $(logthrdest_sources) \ - $(signal_slot_connector_sources) + $(signal_slot_connector_sources) \ + $(perf_sources) lib_libsyslog_ng_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/lib/apphook.c b/lib/apphook.c index b6ce437159..89ebdecb30 100644 --- a/lib/apphook.c +++ b/lib/apphook.c @@ -49,6 +49,7 @@ #include "timeutils/cache.h" #include "multi-line/multi-line-factory.h" #include "filterx/filterx-globals.h" +#include "perf/perf.h" #include #include @@ -215,6 +216,7 @@ construct_nondumpable_logger(msg_fatal); void app_startup(void) { + perf_global_init(); msg_init(FALSE); iv_set_fatal_msg_handler(app_fatal); iv_init(); @@ -293,6 +295,7 @@ app_shutdown(void) hostname_global_deinit(); crypto_deinit(); msg_deinit(); + perf_global_deinit(); /* NOTE: the iv_deinit() call should come here, but there's some exit diff --git a/lib/perf/Makefile.am b/lib/perf/Makefile.am new file mode 100644 index 0000000000..88ad80a99b --- /dev/null +++ b/lib/perf/Makefile.am @@ -0,0 +1,8 @@ +perfincludedir = ${pkgincludedir}/perf + +perfinclude_HEADERS = \ + lib/perf/perf.h + +perf_sources = \ + lib/perf/perf.c \ + lib/perf/trampoline.S diff --git a/lib/perf/perf.c b/lib/perf/perf.c new file mode 100644 index 0000000000..3395aeb7d5 --- /dev/null +++ b/lib/perf/perf.c @@ -0,0 +1,159 @@ +/* + * Copyright (c) 2024 Axoflow + * Copyright (c) 2024 Balázs Scheidler + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ + +#include "perf.h" +#include "messages.h" +#include +#include +#include +#include + +typedef struct _PerfTrampolineArea +{ + guint8 *area; + gsize size; + gsize code_size; + struct + { + gint num; + gint alloc; + } trampolines; +} PerfTrampolineArea; + +extern void _perf_trampoline_func_start(void); +extern void _perf_trampoline_func_end(void); + +#define PAGE_SIZE 4096 +#define PAGE_MAX_INDEX ((PAGE_SIZE-1)) + +#define SIZE_TO_PAGES(s) (((s + PAGE_MAX_INDEX) & ~PAGE_MAX_INDEX) / PAGE_SIZE) +#define PAGE_TO_SIZE(p) (p * PAGE_SIZE) + +#define MAX_TRAMPOLINES 16384 + +static gboolean +_allocate_trampoline_area(PerfTrampolineArea *self) +{ + guint8 *start = (guint8 *) &_perf_trampoline_func_start; + guint8 *end = (guint8 *) &_perf_trampoline_func_end; + + self->code_size = end - start; + gsize code_pages = SIZE_TO_PAGES(self->code_size * MAX_TRAMPOLINES); + + self->size = PAGE_TO_SIZE(code_pages); + self->area = mmap(NULL, + self->size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, + -1, 0); + if (self->area == MAP_FAILED) + return FALSE; + + self->trampolines.num = self->size / self->code_size; + self->trampolines.alloc = 0; + + for (gint i = 0; i < self->trampolines.num; i++) + { + memcpy(self->area + i * self->code_size, start, self->code_size * sizeof(gchar)); + } + return TRUE; +} + +static gpointer +_generate_trampoline(PerfTrampolineArea *self, gpointer target_address) +{ + if (self->trampolines.alloc >= self->trampolines.num) + return NULL; + + gint res = mprotect(self->area, self->size, PROT_READ | PROT_WRITE); + if (res < 0) + return NULL; + + guint8 *trampoline_start = self->area + (self->trampolines.alloc++) * self->code_size; + guint8 *trampoline_end = trampoline_start + self->code_size; + uintptr_t *value_p = (uintptr_t *) (trampoline_end - sizeof(target_address)); + *value_p = (uintptr_t) target_address; + + __builtin___clear_cache((gpointer) trampoline_start, (gpointer) trampoline_end); + + res = mprotect(self->area, self->size, PROT_READ | PROT_EXEC); + if (res < 0) + return NULL; + return (gpointer) trampoline_start; +} + +static gboolean +_is_trampoline_address(PerfTrampolineArea *self, guint8 *address) +{ + return self->area <= address && self->area + self->size > address; +} + +static gboolean +_save_symbol(gpointer address, gsize size, const gchar *symbol_name) +{ + gchar filename[64]; + + g_snprintf(filename, sizeof(filename), "/tmp/perf-%d.map", (int) getpid()); + FILE *mapfile = fopen(filename, "a"); + if (!mapfile) + return FALSE; + fprintf(mapfile, "%0lx %ld %s\n", (uintptr_t) address, size, symbol_name); + fclose(mapfile); + return TRUE; +} + +static PerfTrampolineArea trampolines; + +gpointer +perf_generate_trampoline(gpointer target_address, const gchar *symbol_name) +{ + gpointer t = _generate_trampoline(&trampolines, target_address); + + if (!t) + { + msg_warning_once("WARNING: out of free trampoline slots", + evt_tag_int("max_trampolines", trampolines.trampolines.num)); + return target_address; + } + + _save_symbol(t, trampolines.code_size, symbol_name); + return t; +} + +gboolean +perf_is_trampoline_address(gpointer address) +{ + return _is_trampoline_address(&trampolines, address); +} + +void +perf_global_init(void) +{ + _allocate_trampoline_area(&trampolines); +} + +void +perf_global_deinit(void) +{ + /* we are never freeing the trampolines, as that would invalidate any + * function pointers that point into that area */ +} diff --git a/lib/perf/perf.h b/lib/perf/perf.h new file mode 100644 index 0000000000..342ddcec4e --- /dev/null +++ b/lib/perf/perf.h @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2024 Axoflow + * Copyright (c) 2024 Balázs Scheidler + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + * As an additional exemption you are allowed to compile & link against the + * OpenSSL libraries as published by the OpenSSL project. See the file + * COPYING for details. + * + */ + +#ifndef SYSLOG_NG_PERF_H_INCLUDED +#define SYSLOG_NG_PERF_H_INCLUDED + +#include "syslog-ng.h" + +gpointer perf_generate_trampoline(gpointer target_address, const gchar *symbol_name); +gboolean perf_is_trampoline_address(gpointer address); + + +void perf_global_init(void); +void perf_global_deinit(void); + + +#endif diff --git a/lib/perf/trampoline.S b/lib/perf/trampoline.S new file mode 100644 index 0000000000..4b7abc4a52 --- /dev/null +++ b/lib/perf/trampoline.S @@ -0,0 +1,19 @@ + .text + .global _perf_trampoline_func_start + .align 16 +_perf_trampoline_func_start: +#ifdef __x86_64__ + endbr64 + sub $8, %rsp + mov _perf_trampoline_target_address(%rip), %r11 + call *%r11 + add $8, %rsp + ret + .align 8 +_perf_trampoline_target_address: + .quad 0x0 +#endif // __x86_64__ + + .global _perf_trampoline_func_end +_perf_trampoline_func_end: + .section .note.GNU-stack,"",@progbits From 1431a7a729b3f71a246a587c70b757aafa13fee9 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 22 Dec 2024 23:35:49 +0100 Subject: [PATCH 02/47] filterx: use "type" as a description to FilterXExpr type Signed-off-by: Balazs Scheidler --- lib/filterx/expr-compound.c | 2 +- lib/filterx/expr-condition.c | 2 +- lib/filterx/expr-done.c | 2 +- lib/filterx/expr-drop.c | 2 +- lib/filterx/expr-function.c | 2 +- lib/filterx/expr-generator.c | 4 ++-- lib/filterx/expr-get-subscript.c | 2 +- lib/filterx/expr-getattr.c | 2 +- lib/filterx/expr-literal-generator.c | 2 +- lib/filterx/expr-literal.c | 2 +- lib/filterx/expr-regexp.c | 2 +- lib/filterx/expr-set-subscript.c | 4 ++-- lib/filterx/expr-setattr.c | 4 ++-- lib/filterx/expr-template.c | 2 +- lib/filterx/expr-variable.c | 2 +- lib/filterx/filterx-expr.c | 17 +++++++---------- lib/filterx/filterx-expr.h | 5 ++--- libtest/filterx-lib.c | 2 +- 18 files changed, 28 insertions(+), 32 deletions(-) diff --git a/lib/filterx/expr-compound.c b/lib/filterx/expr-compound.c index 8e343a2b93..d4ca2289af 100644 --- a/lib/filterx/expr-compound.c +++ b/lib/filterx/expr-compound.c @@ -231,7 +231,7 @@ filterx_compound_expr_new(gboolean return_value_of_last_expr) { FilterXCompoundExpr *self = g_new0(FilterXCompoundExpr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "compound"); self->super.eval = _eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-condition.c b/lib/filterx/expr-condition.c index 0ca02a1a95..3ab0a600da 100644 --- a/lib/filterx/expr-condition.c +++ b/lib/filterx/expr-condition.c @@ -212,7 +212,7 @@ FilterXExpr * filterx_conditional_new(FilterXExpr *condition) { FilterXConditional *self = g_new0(FilterXConditional, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "conditional"); self->super.eval = _eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-done.c b/lib/filterx/expr-done.c index 8070f796fa..4de23e0d9a 100644 --- a/lib/filterx/expr-done.c +++ b/lib/filterx/expr-done.c @@ -40,7 +40,7 @@ FilterXExpr * filterx_expr_done(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); - filterx_expr_init_instance(self); + filterx_expr_init_instance(self, "done"); self->eval = _eval; return self; diff --git a/lib/filterx/expr-drop.c b/lib/filterx/expr-drop.c index feb8457b82..662fe1c2d2 100644 --- a/lib/filterx/expr-drop.c +++ b/lib/filterx/expr-drop.c @@ -39,7 +39,7 @@ FilterXExpr * filterx_expr_drop_msg(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); - filterx_expr_init_instance(self); + filterx_expr_init_instance(self, "drop"); self->eval = _eval; return self; diff --git a/lib/filterx/expr-function.c b/lib/filterx/expr-function.c index c745529d73..decab96352 100644 --- a/lib/filterx/expr-function.c +++ b/lib/filterx/expr-function.c @@ -303,7 +303,7 @@ _function_free(FilterXExpr *s) void filterx_function_init_instance(FilterXFunction *s, const gchar *function_name) { - filterx_expr_init_instance(&s->super); + filterx_expr_init_instance(&s->super, "call"); s->function_name = g_strdup_printf("%s()", function_name); s->super.optimize = _function_optimize; s->super.init = _function_init; diff --git a/lib/filterx/expr-generator.c b/lib/filterx/expr-generator.c index 246400f347..6f34599fa0 100644 --- a/lib/filterx/expr-generator.c +++ b/lib/filterx/expr-generator.c @@ -67,7 +67,7 @@ filterx_generator_optimize_method(FilterXExpr *s) void filterx_generator_init_instance(FilterXExpr *s) { - filterx_expr_init_instance(s); + filterx_expr_init_instance(s, "generator"); s->optimize = filterx_generator_optimize_method; s->init = filterx_generator_init_method; s->deinit = filterx_generator_deinit_method; @@ -188,7 +188,7 @@ filterx_generator_create_container_new(FilterXExpr *g, FilterXExpr *fillable_par { FilterXExprGeneratorCreateContainer *self = g_new0(FilterXExprGeneratorCreateContainer, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "create_container"); self->generator = (FilterXExprGenerator *) g; self->fillable_parent = fillable_parent; self->super.optimize = _create_container_optimize; diff --git a/lib/filterx/expr-get-subscript.c b/lib/filterx/expr-get-subscript.c index 13bfe4dd29..2251470500 100644 --- a/lib/filterx/expr-get-subscript.c +++ b/lib/filterx/expr-get-subscript.c @@ -169,7 +169,7 @@ filterx_get_subscript_new(FilterXExpr *operand, FilterXExpr *key) { FilterXGetSubscript *self = g_new0(FilterXGetSubscript, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "get_subscript"); self->super.eval = _eval; self->super.is_set = _isset; self->super.unset = _unset; diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index 825029472c..08dfc6f9d3 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -148,7 +148,7 @@ filterx_getattr_new(FilterXExpr *operand, FilterXString *attr_name) { FilterXGetAttr *self = g_new0(FilterXGetAttr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "getattr"); self->super.eval = _eval; self->super.unset = _unset; self->super.is_set = _isset; diff --git a/lib/filterx/expr-literal-generator.c b/lib/filterx/expr-literal-generator.c index 5776b6760c..ba6f309ef4 100644 --- a/lib/filterx/expr-literal-generator.c +++ b/lib/filterx/expr-literal-generator.c @@ -263,7 +263,7 @@ static void _literal_inner_generator_init_instance(FilterXLiteralInnerGenerator *self, FilterXExpr *root_literal_generator, GList *elements) { - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "literal_inner_generator"); self->super.free_fn = _literal_inner_generator_free; /* diff --git a/lib/filterx/expr-literal.c b/lib/filterx/expr-literal.c index a2c8efb25a..b01351da78 100644 --- a/lib/filterx/expr-literal.c +++ b/lib/filterx/expr-literal.c @@ -49,7 +49,7 @@ filterx_literal_new(FilterXObject *object) { FilterXLiteral *self = g_new0(FilterXLiteral, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "literal"); self->super.eval = _eval; self->super.free_fn = _free; self->object = object; diff --git a/lib/filterx/expr-regexp.c b/lib/filterx/expr-regexp.c index f6ed993d62..b1d0ce9827 100644 --- a/lib/filterx/expr-regexp.c +++ b/lib/filterx/expr-regexp.c @@ -111,7 +111,7 @@ filterx_expr_regexp_match_new(FilterXExpr *lhs, const gchar *pattern) { FilterXExprRegexpMatch *self = g_new0(FilterXExprRegexpMatch, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "regexp_match"); self->super.eval = _regexp_match_eval; self->super.optimize = _regexp_match_optimize; self->super.init = _regexp_match_init; diff --git a/lib/filterx/expr-set-subscript.c b/lib/filterx/expr-set-subscript.c index 040c0650c3..c826c1d0ae 100644 --- a/lib/filterx/expr-set-subscript.c +++ b/lib/filterx/expr-set-subscript.c @@ -222,7 +222,7 @@ filterx_nullv_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXEx { FilterXSetSubscript *self = g_new0(FilterXSetSubscript, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "nullv_set_subscript"); self->super.eval = _nullv_set_subscript_eval; self->super.optimize = _optimize; self->super.init = _init; @@ -240,7 +240,7 @@ filterx_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *ne { FilterXSetSubscript *self = g_new0(FilterXSetSubscript, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "set_subscript"); self->super.eval = _set_subscript_eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-setattr.c b/lib/filterx/expr-setattr.c index c15a2dd5ac..ef0ea51651 100644 --- a/lib/filterx/expr-setattr.c +++ b/lib/filterx/expr-setattr.c @@ -196,7 +196,7 @@ filterx_nullv_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterX { FilterXSetAttr *self = g_new0(FilterXSetAttr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "nullv_setattr"); self->super.eval = _nullv_setattr_eval; self->super.optimize = _optimize; self->super.init = _init; @@ -217,7 +217,7 @@ filterx_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterXExpr * { FilterXSetAttr *self = g_new0(FilterXSetAttr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "setattr"); self->super.eval = _setattr_eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-template.c b/lib/filterx/expr-template.c index f94716ad34..17653e869b 100644 --- a/lib/filterx/expr-template.c +++ b/lib/filterx/expr-template.c @@ -99,7 +99,7 @@ filterx_template_new(LogTemplate *template) { FilterXTemplate *self = g_new0(FilterXTemplate, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "template"); self->super.init = _template_init; self->super.deinit = _template_deinit; self->super.eval = _eval; diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 4c962722d0..577d66600f 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -208,7 +208,7 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) { FilterXVariableExpr *self = g_new0(FilterXVariableExpr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "variable"); self->super.free_fn = _free; self->super.init = _init; self->super.deinit = _deinit; diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index ab21621d03..b53d29aeec 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -118,19 +118,20 @@ filterx_expr_free_method(FilterXExpr *self) } void -filterx_expr_init_instance(FilterXExpr *self) +filterx_expr_init_instance(FilterXExpr *self, const gchar *type) { self->ref_cnt = 1; self->init = filterx_expr_init_method; self->deinit = filterx_expr_deinit_method; self->free_fn = filterx_expr_free_method; + self->type = type; } FilterXExpr * filterx_expr_new(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); - filterx_expr_init_instance(self); + filterx_expr_init_instance(self, "expr"); return self; } @@ -216,14 +217,12 @@ filterx_unary_op_free_method(FilterXExpr *s) void filterx_unary_op_init_instance(FilterXUnaryOp *self, const gchar *name, FilterXExpr *operand) { - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, name); self->super.optimize = filterx_unary_op_optimize_method; self->super.init = filterx_unary_op_init_method; self->super.deinit = filterx_unary_op_deinit_method; self->super.free_fn = filterx_unary_op_free_method; self->operand = operand; - - self->name = name; } void @@ -259,7 +258,7 @@ filterx_binary_op_init_method(FilterXExpr *s, GlobalConfig *cfg) stats_lock(); StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; + StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); stats_unlock(); @@ -274,7 +273,7 @@ filterx_binary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) stats_lock(); StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; + StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); stats_unlock(); @@ -287,7 +286,7 @@ filterx_binary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) void filterx_binary_op_init_instance(FilterXBinaryOp *self, const gchar *name, FilterXExpr *lhs, FilterXExpr *rhs) { - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, name); self->super.optimize = filterx_binary_op_optimize_method; self->super.init = filterx_binary_op_init_method; self->super.deinit = filterx_binary_op_deinit_method; @@ -296,6 +295,4 @@ filterx_binary_op_init_instance(FilterXBinaryOp *self, const gchar *name, Filter g_assert(rhs); self->lhs = lhs; self->rhs = rhs; - - self->name = name; } diff --git a/lib/filterx/filterx-expr.h b/lib/filterx/filterx-expr.h index 0a5388ccfd..51fed2c21d 100644 --- a/lib/filterx/filterx-expr.h +++ b/lib/filterx/filterx-expr.h @@ -56,6 +56,7 @@ struct _FilterXExpr FilterXExpr *(*optimize)(FilterXExpr *self); void (*free_fn)(FilterXExpr *self); + /* type of the expr */ const gchar *type; CFG_LTYPE *lloc; gchar *expr_text; @@ -151,7 +152,7 @@ void filterx_expr_set_location(FilterXExpr *self, CfgLexer *lexer, CFG_LTYPE *ll void filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gchar *text); EVTTAG *filterx_expr_format_location_tag(FilterXExpr *self); FilterXExpr *filterx_expr_optimize(FilterXExpr *self); -void filterx_expr_init_instance(FilterXExpr *self); +void filterx_expr_init_instance(FilterXExpr *self, const gchar *type); FilterXExpr *filterx_expr_new(void); FilterXExpr *filterx_expr_ref(FilterXExpr *self); void filterx_expr_unref(FilterXExpr *self); @@ -190,7 +191,6 @@ typedef struct _FilterXUnaryOp { FilterXExpr super; FilterXExpr *operand; - const gchar *name; } FilterXUnaryOp; FilterXExpr *filterx_unary_op_optimize_method(FilterXExpr *s); @@ -203,7 +203,6 @@ typedef struct _FilterXBinaryOp { FilterXExpr super; FilterXExpr *lhs, *rhs; - const gchar *name; } FilterXBinaryOp; FilterXExpr *filterx_binary_op_optimize_method(FilterXExpr *s); diff --git a/libtest/filterx-lib.c b/libtest/filterx-lib.c index 2b46c74e9f..62c0a5a6c4 100644 --- a/libtest/filterx-lib.c +++ b/libtest/filterx-lib.c @@ -168,7 +168,7 @@ filterx_dummy_error_new(const gchar *msg) { FilterXDummyError *self = g_new0(FilterXDummyError, 1); self->msg = g_strdup(msg); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "dummy"); self->super.eval = _eval; self->super.free_fn = _free; return &self->super; From 5e727d5b6dc644bd496d8fc9e94ace4814b5011e Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Mon, 23 Dec 2024 13:49:31 +0100 Subject: [PATCH 03/47] filterx: initialize filterx eval metrics in FilterXExpr Instead of coding this individually in all FilterXExpr derivatives. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-compound.c | 12 ------- lib/filterx/expr-condition.c | 12 ------- lib/filterx/expr-function.c | 17 ++------- lib/filterx/expr-get-subscript.c | 12 ------- lib/filterx/expr-getattr.c | 12 ------- lib/filterx/expr-set-subscript.c | 12 ------- lib/filterx/expr-setattr.c | 12 ------- lib/filterx/expr-template.c | 30 ---------------- lib/filterx/expr-variable.c | 30 ---------------- lib/filterx/filterx-expr.c | 59 +++++++++++++++++--------------- lib/filterx/filterx-expr.h | 8 ++++- 11 files changed, 41 insertions(+), 175 deletions(-) diff --git a/lib/filterx/expr-compound.c b/lib/filterx/expr-compound.c index d4ca2289af..295efdcbc2 100644 --- a/lib/filterx/expr-compound.c +++ b/lib/filterx/expr-compound.c @@ -168,12 +168,6 @@ _init(FilterXExpr *s, GlobalConfig *cfg) } } - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_compound_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -182,12 +176,6 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) { FilterXCompoundExpr *self = (FilterXCompoundExpr *) s; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_compound_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - for (gint i = 0; i < self->exprs->len; i++) { FilterXExpr *expr = g_ptr_array_index(self->exprs, i); diff --git a/lib/filterx/expr-condition.c b/lib/filterx/expr-condition.c index 3ab0a600da..8c0776a2c3 100644 --- a/lib/filterx/expr-condition.c +++ b/lib/filterx/expr-condition.c @@ -60,12 +60,6 @@ _init(FilterXExpr *s, GlobalConfig *cfg) return FALSE; } - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_condition_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -74,12 +68,6 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) { FilterXConditional *self = (FilterXConditional *) s; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_condition_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->condition, cfg); filterx_expr_deinit(self->true_branch, cfg); filterx_expr_deinit(self->false_branch, cfg); diff --git a/lib/filterx/expr-function.c b/lib/filterx/expr-function.c index decab96352..ccacd9ff1c 100644 --- a/lib/filterx/expr-function.c +++ b/lib/filterx/expr-function.c @@ -242,26 +242,12 @@ filterx_function_optimize_method(FilterXFunction *s) gboolean filterx_function_init_method(FilterXFunction *s, GlobalConfig *cfg) { - stats_lock(); - StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", s->function_name) }; - stats_cluster_single_key_set(&sc_key, "fx_func_evals_total", labels, G_N_ELEMENTS(labels)); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &s->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(&s->super, cfg); } void filterx_function_deinit_method(FilterXFunction *s, GlobalConfig *cfg) { - stats_lock(); - StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", s->function_name) }; - stats_cluster_single_key_set(&sc_key, "fx_func_evals_total", labels, G_N_ELEMENTS(labels)); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &s->super.eval_count); - stats_unlock(); - filterx_expr_deinit_method(&s->super, cfg); } @@ -303,11 +289,12 @@ _function_free(FilterXExpr *s) void filterx_function_init_instance(FilterXFunction *s, const gchar *function_name) { - filterx_expr_init_instance(&s->super, "call"); + filterx_expr_init_instance(&s->super, "func"); s->function_name = g_strdup_printf("%s()", function_name); s->super.optimize = _function_optimize; s->super.init = _function_init; s->super.deinit = _function_deinit; + s->super.name = s->function_name; s->super.free_fn = _function_free; } diff --git a/lib/filterx/expr-get-subscript.c b/lib/filterx/expr-get-subscript.c index 2251470500..ac831a8799 100644 --- a/lib/filterx/expr-get-subscript.c +++ b/lib/filterx/expr-get-subscript.c @@ -129,12 +129,6 @@ _init(FilterXExpr *s, GlobalConfig *cfg) return FALSE; } - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_get_subscript_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -143,12 +137,6 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) { FilterXGetSubscript *self = (FilterXGetSubscript *) s; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_get_subscript_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->operand, cfg); filterx_expr_deinit(self->key, cfg); filterx_expr_deinit_method(s, cfg); diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index 08dfc6f9d3..f073a524fd 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -109,12 +109,6 @@ _init(FilterXExpr *s, GlobalConfig *cfg) if (!filterx_expr_init(self->operand, cfg)) return FALSE; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_getattr_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -123,12 +117,6 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) { FilterXGetAttr *self = (FilterXGetAttr *) s; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_getattr_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->operand, cfg); filterx_expr_deinit_method(s, cfg); } diff --git a/lib/filterx/expr-set-subscript.c b/lib/filterx/expr-set-subscript.c index c826c1d0ae..dbcb9e1c79 100644 --- a/lib/filterx/expr-set-subscript.c +++ b/lib/filterx/expr-set-subscript.c @@ -180,12 +180,6 @@ _init(FilterXExpr *s, GlobalConfig *cfg) return FALSE; } - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_set_subscript_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -194,12 +188,6 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) { FilterXSetSubscript *self = (FilterXSetSubscript *) s; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_set_subscript_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->object, cfg); filterx_expr_deinit(self->new_value, cfg); filterx_expr_deinit(self->key, cfg); diff --git a/lib/filterx/expr-setattr.c b/lib/filterx/expr-setattr.c index ef0ea51651..29adb7ae72 100644 --- a/lib/filterx/expr-setattr.c +++ b/lib/filterx/expr-setattr.c @@ -155,12 +155,6 @@ _init(FilterXExpr *s, GlobalConfig *cfg) return FALSE; } - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_setattr_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -169,12 +163,6 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) { FilterXSetAttr *self = (FilterXSetAttr *) s; - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_setattr_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->object, cfg); filterx_expr_deinit(self->new_value, cfg); filterx_expr_deinit_method(s, cfg); diff --git a/lib/filterx/expr-template.c b/lib/filterx/expr-template.c index 17653e869b..d37560cc39 100644 --- a/lib/filterx/expr-template.c +++ b/lib/filterx/expr-template.c @@ -65,34 +65,6 @@ _free(FilterXExpr *s) filterx_expr_free_method(s); } -static gboolean -_template_init(FilterXExpr *s, GlobalConfig *cfg) -{ - FilterXTemplate *self = (FilterXTemplate *) s; - - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_template_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - - return filterx_expr_init_method(s, cfg); -} - -static void -_template_deinit(FilterXExpr *s, GlobalConfig *cfg) -{ - FilterXTemplate *self = (FilterXTemplate *) s; - - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_template_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - - filterx_expr_deinit_method(s, cfg); -} - /* NOTE: takes the object reference */ FilterXExpr * filterx_template_new(LogTemplate *template) @@ -100,8 +72,6 @@ filterx_template_new(LogTemplate *template) FilterXTemplate *self = g_new0(FilterXTemplate, 1); filterx_expr_init_instance(&self->super, "template"); - self->super.init = _template_init; - self->super.deinit = _template_deinit; self->super.eval = _eval; self->super.free_fn = _free; self->template = template; diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 577d66600f..2d5bb70e21 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -175,34 +175,6 @@ _free(FilterXExpr *s) filterx_expr_free_method(s); } -static gboolean -_init(FilterXExpr *s, GlobalConfig *cfg) -{ - FilterXVariableExpr *self = (FilterXVariableExpr *) s; - - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_variable_evals_total", NULL, 0); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - - return filterx_expr_init_method(s, cfg); -} - -static void -_deinit(FilterXExpr *s, GlobalConfig *cfg) -{ - FilterXVariableExpr *self = (FilterXVariableExpr *) s; - - stats_lock(); - StatsClusterKey sc_key; - stats_cluster_single_key_set(&sc_key, "fx_variable_evals_total", NULL, 0); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - - return filterx_expr_deinit_method(s, cfg); -} - static FilterXExpr * filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) { @@ -210,8 +182,6 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) filterx_expr_init_instance(&self->super, "variable"); self->super.free_fn = _free; - self->super.init = _init; - self->super.deinit = _deinit; self->super.eval = _eval; self->super._update_repr = _update_repr; self->super.assign = _assign; diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index b53d29aeec..29e61e24d5 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -99,15 +99,47 @@ filterx_expr_optimize(FilterXExpr *self) return optimized; } +static void +_init_sc_key_name(FilterXExpr *self, gchar *buf, gsize buf_len) +{ + g_snprintf(buf, buf_len, "fx_%s_evals_total", self->type); +} + gboolean filterx_expr_init_method(FilterXExpr *self, GlobalConfig *cfg) { + gchar buf[64]; + + _init_sc_key_name(self, buf, sizeof(buf)); + stats_lock(); + StatsClusterKey sc_key; + StatsClusterLabel labels[1]; + gint labels_len = 0; + + if (self->name) + labels[labels_len++] = stats_cluster_label("name", self->name); + stats_cluster_single_key_set(&sc_key, buf, labels, labels_len); + stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->eval_count); + stats_unlock(); return TRUE; } void filterx_expr_deinit_method(FilterXExpr *self, GlobalConfig *cfg) { + gchar buf[64]; + + _init_sc_key_name(self, buf, sizeof(buf)); + stats_lock(); + StatsClusterKey sc_key; + StatsClusterLabel labels[1]; + gint labels_len = 0; + + if (self->name) + labels[labels_len++] = stats_cluster_label("name", self->name); + stats_cluster_single_key_set(&sc_key, buf, labels, labels_len); + stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->eval_count); + stats_unlock(); } void @@ -179,12 +211,6 @@ filterx_unary_op_init_method(FilterXExpr *s, GlobalConfig *cfg) if (!filterx_expr_init(self->operand, cfg)) return FALSE; - stats_lock(); - StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; - stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); return filterx_expr_init_method(s, cfg); } @@ -194,13 +220,6 @@ filterx_unary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) { FilterXUnaryOp *self = (FilterXUnaryOp *) s; - stats_lock(); - StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; - stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->operand, cfg); filterx_expr_deinit_method(s, cfg); } @@ -256,13 +275,6 @@ filterx_binary_op_init_method(FilterXExpr *s, GlobalConfig *cfg) if (!filterx_expr_init(self->rhs, cfg)) return FALSE; - stats_lock(); - StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; - stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); - stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - return filterx_expr_init_method(s, cfg); } @@ -271,13 +283,6 @@ filterx_binary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) { FilterXBinaryOp *self = (FilterXBinaryOp *) s; - stats_lock(); - StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; - stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); - stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); - stats_unlock(); - filterx_expr_deinit(self->lhs, cfg); filterx_expr_deinit(self->rhs, cfg); filterx_expr_deinit_method(s, cfg); diff --git a/lib/filterx/filterx-expr.h b/lib/filterx/filterx-expr.h index 51fed2c21d..8ce94590d0 100644 --- a/lib/filterx/filterx-expr.h +++ b/lib/filterx/filterx-expr.h @@ -56,8 +56,14 @@ struct _FilterXExpr FilterXExpr *(*optimize)(FilterXExpr *self); void (*free_fn)(FilterXExpr *self); - /* type of the expr */ + /* type of the expr, is not freed, assumed to be managed by something else + * */ + const gchar *type; + + /* name associated with the expr (e.g. function name), is not freed by + * FilterXExpr, assumed to be managed by something else */ + const gchar *name; CFG_LTYPE *lloc; gchar *expr_text; }; From a1931a1f3218f9522624e51c6a0c354cf84a6afa Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Mon, 23 Dec 2024 14:51:10 +0100 Subject: [PATCH 04/47] filterx: store "name" attribute extra granularity in metrics Signed-off-by: Balazs Scheidler --- lib/filterx/expr-getattr.c | 3 ++- lib/filterx/expr-setattr.c | 2 ++ lib/filterx/expr-variable.c | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index f073a524fd..6bd0c48bcf 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -147,6 +147,7 @@ filterx_getattr_new(FilterXExpr *operand, FilterXString *attr_name) self->operand = operand; self->attr = (FilterXObject *) attr_name; - + /* NOTE: name borrows the string value from the string object */ + self->super.name = filterx_string_get_value_ref(self->attr, NULL); return &self->super; } diff --git a/lib/filterx/expr-setattr.c b/lib/filterx/expr-setattr.c index 29adb7ae72..aaa21ae869 100644 --- a/lib/filterx/expr-setattr.c +++ b/lib/filterx/expr-setattr.c @@ -196,6 +196,8 @@ filterx_nullv_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterX self->new_value = new_value; self->super.ignore_falsy_result = TRUE; + /* NOTE: name borrows the string value from the string object */ + self->super.name = filterx_string_get_value_ref(self->attr, NULL); return &self->super; } diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 2d5bb70e21..b904cd310f 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -193,6 +193,9 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) if (type == FX_VAR_MESSAGE) self->handle_is_macro = log_msg_is_handle_macro(filterx_variable_handle_to_nv_handle(self->handle)); + /* NOTE: name borrows the string value from the string object */ + self->super.name = filterx_string_get_value_ref(self->variable_name, NULL); + return &self->super; } From a3a47ff5fde5aa58ac40f1df35d3f5c0966e6ee4 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 22 Dec 2024 23:35:49 +0100 Subject: [PATCH 05/47] filterx: use "type" as a description to FilterXExpr type Signed-off-by: Balazs Scheidler --- lib/filterx/expr-compound.c | 2 +- lib/filterx/expr-condition.c | 2 +- lib/filterx/expr-done.c | 2 +- lib/filterx/expr-drop.c | 2 +- lib/filterx/expr-function.c | 2 +- lib/filterx/expr-generator.c | 4 ++-- lib/filterx/expr-get-subscript.c | 2 +- lib/filterx/expr-getattr.c | 2 +- lib/filterx/expr-literal-generator.c | 2 +- lib/filterx/expr-literal.c | 2 +- lib/filterx/expr-regexp.c | 2 +- lib/filterx/expr-set-subscript.c | 4 ++-- lib/filterx/expr-setattr.c | 4 ++-- lib/filterx/expr-template.c | 2 +- lib/filterx/expr-variable.c | 2 +- lib/filterx/filterx-expr.c | 21 +++++++++------------ lib/filterx/filterx-expr.h | 5 ++--- libtest/filterx-lib.c | 2 +- 18 files changed, 30 insertions(+), 34 deletions(-) diff --git a/lib/filterx/expr-compound.c b/lib/filterx/expr-compound.c index 8e343a2b93..d4ca2289af 100644 --- a/lib/filterx/expr-compound.c +++ b/lib/filterx/expr-compound.c @@ -231,7 +231,7 @@ filterx_compound_expr_new(gboolean return_value_of_last_expr) { FilterXCompoundExpr *self = g_new0(FilterXCompoundExpr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "compound"); self->super.eval = _eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-condition.c b/lib/filterx/expr-condition.c index 0ca02a1a95..3ab0a600da 100644 --- a/lib/filterx/expr-condition.c +++ b/lib/filterx/expr-condition.c @@ -212,7 +212,7 @@ FilterXExpr * filterx_conditional_new(FilterXExpr *condition) { FilterXConditional *self = g_new0(FilterXConditional, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "conditional"); self->super.eval = _eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-done.c b/lib/filterx/expr-done.c index 8070f796fa..4de23e0d9a 100644 --- a/lib/filterx/expr-done.c +++ b/lib/filterx/expr-done.c @@ -40,7 +40,7 @@ FilterXExpr * filterx_expr_done(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); - filterx_expr_init_instance(self); + filterx_expr_init_instance(self, "done"); self->eval = _eval; return self; diff --git a/lib/filterx/expr-drop.c b/lib/filterx/expr-drop.c index feb8457b82..662fe1c2d2 100644 --- a/lib/filterx/expr-drop.c +++ b/lib/filterx/expr-drop.c @@ -39,7 +39,7 @@ FilterXExpr * filterx_expr_drop_msg(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); - filterx_expr_init_instance(self); + filterx_expr_init_instance(self, "drop"); self->eval = _eval; return self; diff --git a/lib/filterx/expr-function.c b/lib/filterx/expr-function.c index c745529d73..decab96352 100644 --- a/lib/filterx/expr-function.c +++ b/lib/filterx/expr-function.c @@ -303,7 +303,7 @@ _function_free(FilterXExpr *s) void filterx_function_init_instance(FilterXFunction *s, const gchar *function_name) { - filterx_expr_init_instance(&s->super); + filterx_expr_init_instance(&s->super, "call"); s->function_name = g_strdup_printf("%s()", function_name); s->super.optimize = _function_optimize; s->super.init = _function_init; diff --git a/lib/filterx/expr-generator.c b/lib/filterx/expr-generator.c index 246400f347..6f34599fa0 100644 --- a/lib/filterx/expr-generator.c +++ b/lib/filterx/expr-generator.c @@ -67,7 +67,7 @@ filterx_generator_optimize_method(FilterXExpr *s) void filterx_generator_init_instance(FilterXExpr *s) { - filterx_expr_init_instance(s); + filterx_expr_init_instance(s, "generator"); s->optimize = filterx_generator_optimize_method; s->init = filterx_generator_init_method; s->deinit = filterx_generator_deinit_method; @@ -188,7 +188,7 @@ filterx_generator_create_container_new(FilterXExpr *g, FilterXExpr *fillable_par { FilterXExprGeneratorCreateContainer *self = g_new0(FilterXExprGeneratorCreateContainer, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "create_container"); self->generator = (FilterXExprGenerator *) g; self->fillable_parent = fillable_parent; self->super.optimize = _create_container_optimize; diff --git a/lib/filterx/expr-get-subscript.c b/lib/filterx/expr-get-subscript.c index 13bfe4dd29..2251470500 100644 --- a/lib/filterx/expr-get-subscript.c +++ b/lib/filterx/expr-get-subscript.c @@ -169,7 +169,7 @@ filterx_get_subscript_new(FilterXExpr *operand, FilterXExpr *key) { FilterXGetSubscript *self = g_new0(FilterXGetSubscript, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "get_subscript"); self->super.eval = _eval; self->super.is_set = _isset; self->super.unset = _unset; diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index 825029472c..08dfc6f9d3 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -148,7 +148,7 @@ filterx_getattr_new(FilterXExpr *operand, FilterXString *attr_name) { FilterXGetAttr *self = g_new0(FilterXGetAttr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "getattr"); self->super.eval = _eval; self->super.unset = _unset; self->super.is_set = _isset; diff --git a/lib/filterx/expr-literal-generator.c b/lib/filterx/expr-literal-generator.c index 5776b6760c..ba6f309ef4 100644 --- a/lib/filterx/expr-literal-generator.c +++ b/lib/filterx/expr-literal-generator.c @@ -263,7 +263,7 @@ static void _literal_inner_generator_init_instance(FilterXLiteralInnerGenerator *self, FilterXExpr *root_literal_generator, GList *elements) { - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "literal_inner_generator"); self->super.free_fn = _literal_inner_generator_free; /* diff --git a/lib/filterx/expr-literal.c b/lib/filterx/expr-literal.c index a2c8efb25a..b01351da78 100644 --- a/lib/filterx/expr-literal.c +++ b/lib/filterx/expr-literal.c @@ -49,7 +49,7 @@ filterx_literal_new(FilterXObject *object) { FilterXLiteral *self = g_new0(FilterXLiteral, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "literal"); self->super.eval = _eval; self->super.free_fn = _free; self->object = object; diff --git a/lib/filterx/expr-regexp.c b/lib/filterx/expr-regexp.c index f6ed993d62..b1d0ce9827 100644 --- a/lib/filterx/expr-regexp.c +++ b/lib/filterx/expr-regexp.c @@ -111,7 +111,7 @@ filterx_expr_regexp_match_new(FilterXExpr *lhs, const gchar *pattern) { FilterXExprRegexpMatch *self = g_new0(FilterXExprRegexpMatch, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "regexp_match"); self->super.eval = _regexp_match_eval; self->super.optimize = _regexp_match_optimize; self->super.init = _regexp_match_init; diff --git a/lib/filterx/expr-set-subscript.c b/lib/filterx/expr-set-subscript.c index 040c0650c3..c826c1d0ae 100644 --- a/lib/filterx/expr-set-subscript.c +++ b/lib/filterx/expr-set-subscript.c @@ -222,7 +222,7 @@ filterx_nullv_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXEx { FilterXSetSubscript *self = g_new0(FilterXSetSubscript, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "nullv_set_subscript"); self->super.eval = _nullv_set_subscript_eval; self->super.optimize = _optimize; self->super.init = _init; @@ -240,7 +240,7 @@ filterx_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *ne { FilterXSetSubscript *self = g_new0(FilterXSetSubscript, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "set_subscript"); self->super.eval = _set_subscript_eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-setattr.c b/lib/filterx/expr-setattr.c index c15a2dd5ac..ef0ea51651 100644 --- a/lib/filterx/expr-setattr.c +++ b/lib/filterx/expr-setattr.c @@ -196,7 +196,7 @@ filterx_nullv_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterX { FilterXSetAttr *self = g_new0(FilterXSetAttr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "nullv_setattr"); self->super.eval = _nullv_setattr_eval; self->super.optimize = _optimize; self->super.init = _init; @@ -217,7 +217,7 @@ filterx_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterXExpr * { FilterXSetAttr *self = g_new0(FilterXSetAttr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "setattr"); self->super.eval = _setattr_eval; self->super.optimize = _optimize; self->super.init = _init; diff --git a/lib/filterx/expr-template.c b/lib/filterx/expr-template.c index f94716ad34..17653e869b 100644 --- a/lib/filterx/expr-template.c +++ b/lib/filterx/expr-template.c @@ -99,7 +99,7 @@ filterx_template_new(LogTemplate *template) { FilterXTemplate *self = g_new0(FilterXTemplate, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "template"); self->super.init = _template_init; self->super.deinit = _template_deinit; self->super.eval = _eval; diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 4c962722d0..577d66600f 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -208,7 +208,7 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) { FilterXVariableExpr *self = g_new0(FilterXVariableExpr, 1); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "variable"); self->super.free_fn = _free; self->super.init = _init; self->super.deinit = _deinit; diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index ab21621d03..0dfebe0c72 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -118,19 +118,20 @@ filterx_expr_free_method(FilterXExpr *self) } void -filterx_expr_init_instance(FilterXExpr *self) +filterx_expr_init_instance(FilterXExpr *self, const gchar *type) { self->ref_cnt = 1; self->init = filterx_expr_init_method; self->deinit = filterx_expr_deinit_method; self->free_fn = filterx_expr_free_method; + self->type = type; } FilterXExpr * filterx_expr_new(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); - filterx_expr_init_instance(self); + filterx_expr_init_instance(self, "expr"); return self; } @@ -180,7 +181,7 @@ filterx_unary_op_init_method(FilterXExpr *s, GlobalConfig *cfg) stats_lock(); StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; + StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); stats_unlock(); @@ -195,7 +196,7 @@ filterx_unary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) stats_lock(); StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; + StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); stats_unlock(); @@ -216,14 +217,12 @@ filterx_unary_op_free_method(FilterXExpr *s) void filterx_unary_op_init_instance(FilterXUnaryOp *self, const gchar *name, FilterXExpr *operand) { - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, name); self->super.optimize = filterx_unary_op_optimize_method; self->super.init = filterx_unary_op_init_method; self->super.deinit = filterx_unary_op_deinit_method; self->super.free_fn = filterx_unary_op_free_method; self->operand = operand; - - self->name = name; } void @@ -259,7 +258,7 @@ filterx_binary_op_init_method(FilterXExpr *s, GlobalConfig *cfg) stats_lock(); StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; + StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); stats_unlock(); @@ -274,7 +273,7 @@ filterx_binary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) stats_lock(); StatsClusterKey sc_key; - StatsClusterLabel labels[] = { stats_cluster_label("name", self->name) }; + StatsClusterLabel labels[] = { stats_cluster_label("name", self->super.type) }; stats_cluster_single_key_set(&sc_key, "fx_op_evals_total", labels, G_N_ELEMENTS(labels)); stats_unregister_counter(&sc_key, SC_TYPE_SINGLE_VALUE, &self->super.eval_count); stats_unlock(); @@ -287,7 +286,7 @@ filterx_binary_op_deinit_method(FilterXExpr *s, GlobalConfig *cfg) void filterx_binary_op_init_instance(FilterXBinaryOp *self, const gchar *name, FilterXExpr *lhs, FilterXExpr *rhs) { - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, name); self->super.optimize = filterx_binary_op_optimize_method; self->super.init = filterx_binary_op_init_method; self->super.deinit = filterx_binary_op_deinit_method; @@ -296,6 +295,4 @@ filterx_binary_op_init_instance(FilterXBinaryOp *self, const gchar *name, Filter g_assert(rhs); self->lhs = lhs; self->rhs = rhs; - - self->name = name; } diff --git a/lib/filterx/filterx-expr.h b/lib/filterx/filterx-expr.h index 0a5388ccfd..51fed2c21d 100644 --- a/lib/filterx/filterx-expr.h +++ b/lib/filterx/filterx-expr.h @@ -56,6 +56,7 @@ struct _FilterXExpr FilterXExpr *(*optimize)(FilterXExpr *self); void (*free_fn)(FilterXExpr *self); + /* type of the expr */ const gchar *type; CFG_LTYPE *lloc; gchar *expr_text; @@ -151,7 +152,7 @@ void filterx_expr_set_location(FilterXExpr *self, CfgLexer *lexer, CFG_LTYPE *ll void filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gchar *text); EVTTAG *filterx_expr_format_location_tag(FilterXExpr *self); FilterXExpr *filterx_expr_optimize(FilterXExpr *self); -void filterx_expr_init_instance(FilterXExpr *self); +void filterx_expr_init_instance(FilterXExpr *self, const gchar *type); FilterXExpr *filterx_expr_new(void); FilterXExpr *filterx_expr_ref(FilterXExpr *self); void filterx_expr_unref(FilterXExpr *self); @@ -190,7 +191,6 @@ typedef struct _FilterXUnaryOp { FilterXExpr super; FilterXExpr *operand; - const gchar *name; } FilterXUnaryOp; FilterXExpr *filterx_unary_op_optimize_method(FilterXExpr *s); @@ -203,7 +203,6 @@ typedef struct _FilterXBinaryOp { FilterXExpr super; FilterXExpr *lhs, *rhs; - const gchar *name; } FilterXBinaryOp; FilterXExpr *filterx_binary_op_optimize_method(FilterXExpr *s); diff --git a/libtest/filterx-lib.c b/libtest/filterx-lib.c index 2b46c74e9f..62c0a5a6c4 100644 --- a/libtest/filterx-lib.c +++ b/libtest/filterx-lib.c @@ -168,7 +168,7 @@ filterx_dummy_error_new(const gchar *msg) { FilterXDummyError *self = g_new0(FilterXDummyError, 1); self->msg = g_strdup(msg); - filterx_expr_init_instance(&self->super); + filterx_expr_init_instance(&self->super, "dummy"); self->super.eval = _eval; self->super.free_fn = _free; return &self->super; From ce9aa7e2ce121d6637def16919eeecf0d77045bf Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 28 Dec 2024 17:57:17 +0100 Subject: [PATCH 06/47] filterx: fix white space issues in the grammar Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-grammar.ym | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/filterx/filterx-grammar.ym b/lib/filterx/filterx-grammar.ym index 6c24963e36..61f850e698 100644 --- a/lib/filterx/filterx-grammar.ym +++ b/lib/filterx/filterx-grammar.ym @@ -218,15 +218,18 @@ plus_assignment | expr '[' expr ']' KW_PLUS_ASSIGN expr { $$ = filterx_set_subscript_new(filterx_expr_ref($1), filterx_expr_ref($3), filterx_operator_plus_new(filterx_get_subscript_new($1, $3), $6)); } | expr '.' identifier KW_PLUS_ASSIGN expr { - $$ = filterx_setattr_new(filterx_expr_ref($1), filterx_config_frozen_string(configuration, $3), filterx_operator_plus_new(filterx_getattr_new($1, filterx_config_frozen_string(configuration, $3)), $5)); - free($3); + $$ = filterx_setattr_new(filterx_expr_ref($1), + filterx_config_frozen_string(configuration, $3), + filterx_operator_plus_new(filterx_getattr_new($1, filterx_config_frozen_string(configuration, $3)), + $5)); + free($3); } ; assignment /* TODO extract lvalues */ - : variable KW_ASSIGN expr { $$ = filterx_assign_new($1, $3); } + : variable KW_ASSIGN expr { $$ = filterx_assign_new($1, $3); } | expr '.' identifier KW_ASSIGN expr { $$ = filterx_setattr_new($1, filterx_config_frozen_string(configuration, $3), $5); free($3); } | expr '[' expr ']' KW_ASSIGN expr { $$ = filterx_set_subscript_new($1, $3, $6); } | expr '[' ']' KW_ASSIGN expr { $$ = filterx_set_subscript_new($1, NULL, $5); } @@ -308,7 +311,7 @@ generator_assignment ; generator_plus_assignment - : variable KW_PLUS_ASSIGN expr_generator { $$ = $3; filterx_generator_set_fillable($3, $1); } + : variable KW_PLUS_ASSIGN expr_generator { $$ = $3; filterx_generator_set_fillable($3, $1); } | expr '[' expr ']' KW_PLUS_ASSIGN expr_generator { $$ = $6; filterx_generator_set_fillable($6, filterx_get_subscript_new($1, $3)); } | expr '.' identifier KW_PLUS_ASSIGN expr_generator { $$ = $5; filterx_generator_set_fillable($5, filterx_getattr_new($1, filterx_config_frozen_string(configuration, $3))); free($3);} @@ -397,16 +400,16 @@ declaration } ; -expr: __expr { $$ = $1; filterx_expr_set_location($1, lexer, &@1); } +expr: __expr { $$ = _assign_location($1, lexer, &@1); } __expr : expr_value | function_call | expr_operator - | '(' expr ')' { $$ = $2; } - | KW_ISSET '(' expr ')' { $$ = filterx_isset_new($3); } - | KW_DROP { $$ = filterx_expr_drop_msg(); } - | KW_DONE { $$ = filterx_expr_done(); } + | '(' expr ')' { $$ = $2; } + | KW_ISSET '(' expr ')' { $$ = filterx_isset_new($3); } + | KW_DROP { $$ = filterx_expr_drop_msg(); } + | KW_DONE { $$ = filterx_expr_done(); } ; expr_operator @@ -417,7 +420,7 @@ expr_operator | default /* TODO extract lvalues */ | expr '.' identifier { $$ = filterx_getattr_new($1, filterx_config_frozen_string(configuration, $3)); free($3); } - | expr '[' expr ']' { $$ = filterx_get_subscript_new($1, $3); } + | expr '[' expr ']' { $$ = filterx_get_subscript_new($1, $3); } ; boolalg_operator From 772562a25137939b8332434d000f5978e6921adb Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Mon, 23 Dec 2024 13:49:24 +0100 Subject: [PATCH 07/47] filterx: add type information into optimization message Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-expr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index 0dfebe0c72..897a38d4c0 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -88,7 +88,9 @@ filterx_expr_optimize(FilterXExpr *self) optimized = filterx_expr_optimize(optimized); msg_trace("FilterX: expression optimized", - filterx_expr_format_location_tag(self)); + filterx_expr_format_location_tag(self), + evt_tag_str("old_type", self->type), + evt_tag_str("new_type", optimized->type)); if (self->lloc) { /* copy location information to the optimized representation */ From 719cce37c1dde8998a15166ceb375842f4560f7f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 22:38:50 +0100 Subject: [PATCH 08/47] filterx/filterx-ref: move filterx_ref_new() fastpath to an inline function Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-ref.c | 6 ++---- lib/filterx/filterx-ref.h | 12 ++++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/filterx/filterx-ref.c b/lib/filterx/filterx-ref.c index 08f7b80522..75f155c08b 100644 --- a/lib/filterx/filterx-ref.c +++ b/lib/filterx/filterx-ref.c @@ -238,12 +238,10 @@ _filterx_ref_add(FilterXObject *s, FilterXObject *object) return filterx_object_add_object(self->value, object); } +/* NOTE: fastpath is in the header as an inline function */ FilterXObject * -filterx_ref_new(FilterXObject *value) +_filterx_ref_new(FilterXObject *value) { - if (!value || value->readonly || !_filterx_type_is_referenceable(value->type)) - return value; - if (filterx_object_is_type(value, &FILTERX_TYPE_NAME(ref))) { FilterXRef *absorbed_ref = (FilterXRef *) value; diff --git a/lib/filterx/filterx-ref.h b/lib/filterx/filterx-ref.h index 9e68cfe72c..88d5d6035f 100644 --- a/lib/filterx/filterx-ref.h +++ b/lib/filterx/filterx-ref.h @@ -48,8 +48,6 @@ typedef struct _FilterXRef } FilterXRef; -FilterXObject *filterx_ref_new(FilterXObject *value); - /* Call them only where downcasting to a specific type is needed, the returned object should only be used locally. */ FilterXObject *filterx_ref_unwrap_ro(FilterXObject *s); FilterXObject *filterx_ref_unwrap_rw(FilterXObject *s); @@ -60,4 +58,14 @@ _filterx_type_is_referenceable(FilterXType *t) return t->is_mutable && t != &FILTERX_TYPE_NAME(ref); } +FilterXObject *_filterx_ref_new(FilterXObject *value); + +static inline FilterXObject * +filterx_ref_new(FilterXObject *value) +{ + if (!value || value->readonly || !_filterx_type_is_referenceable(value->type)) + return value; + return _filterx_ref_new(value); +} + #endif From 4c9e715c15777113036a52cfae56c457a43cb989 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Mon, 23 Dec 2024 14:50:27 +0100 Subject: [PATCH 09/47] filterx: simplify constructors for nullv assignments Signed-off-by: Balazs Scheidler --- lib/filterx/expr-assign.c | 19 +++++++++---------- lib/filterx/expr-set-subscript.c | 24 ++++++++---------------- lib/filterx/expr-setattr.c | 29 +++++++++-------------------- 3 files changed, 26 insertions(+), 46 deletions(-) diff --git a/lib/filterx/expr-assign.c b/lib/filterx/expr-assign.c index 0f4ff3baac..55581d8fe0 100644 --- a/lib/filterx/expr-assign.c +++ b/lib/filterx/expr-assign.c @@ -91,16 +91,6 @@ _assign_eval(FilterXExpr *s) return _assign(self, value); } -FilterXExpr * -filterx_nullv_assign_new(FilterXExpr *lhs, FilterXExpr *rhs) -{ - FilterXBinaryOp *self = g_new0(FilterXBinaryOp, 1); - - filterx_binary_op_init_instance(self, "nullv_assign", lhs, rhs); - self->super.eval = _nullv_assign_eval; - self->super.ignore_falsy_result = TRUE; - return &self->super; -} /* NOTE: takes the object reference */ FilterXExpr * @@ -113,3 +103,12 @@ filterx_assign_new(FilterXExpr *lhs, FilterXExpr *rhs) self->super.ignore_falsy_result = TRUE; return &self->super; } + +FilterXExpr * +filterx_nullv_assign_new(FilterXExpr *lhs, FilterXExpr *rhs) +{ + FilterXExpr *self = filterx_assign_new(lhs, rhs); + self->type = "nullv_assign"; + self->eval = _nullv_assign_eval; + return self; +} diff --git a/lib/filterx/expr-set-subscript.c b/lib/filterx/expr-set-subscript.c index c826c1d0ae..0a8cb12dae 100644 --- a/lib/filterx/expr-set-subscript.c +++ b/lib/filterx/expr-set-subscript.c @@ -218,12 +218,12 @@ _free(FilterXExpr *s) } FilterXExpr * -filterx_nullv_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *new_value) +filterx_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *new_value) { FilterXSetSubscript *self = g_new0(FilterXSetSubscript, 1); - filterx_expr_init_instance(&self->super, "nullv_set_subscript"); - self->super.eval = _nullv_set_subscript_eval; + filterx_expr_init_instance(&self->super, "set_subscript"); + self->super.eval = _set_subscript_eval; self->super.optimize = _optimize; self->super.init = _init; self->super.deinit = _deinit; @@ -236,19 +236,11 @@ filterx_nullv_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXEx } FilterXExpr * -filterx_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *new_value) +filterx_nullv_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *new_value) { - FilterXSetSubscript *self = g_new0(FilterXSetSubscript, 1); + FilterXExpr *self = filterx_set_subscript_new(object, key, new_value); - filterx_expr_init_instance(&self->super, "set_subscript"); - self->super.eval = _set_subscript_eval; - self->super.optimize = _optimize; - self->super.init = _init; - self->super.deinit = _deinit; - self->super.free_fn = _free; - self->object = object; - self->key = key; - self->new_value = new_value; - self->super.ignore_falsy_result = TRUE; - return &self->super; + self->type = "nullv_set_subscript"; + self->eval = _nullv_set_subscript_eval; + return self; } diff --git a/lib/filterx/expr-setattr.c b/lib/filterx/expr-setattr.c index ef0ea51651..731cd0f69f 100644 --- a/lib/filterx/expr-setattr.c +++ b/lib/filterx/expr-setattr.c @@ -191,26 +191,6 @@ _free(FilterXExpr *s) filterx_expr_free_method(s); } -FilterXExpr * -filterx_nullv_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterXExpr *new_value) -{ - FilterXSetAttr *self = g_new0(FilterXSetAttr, 1); - - filterx_expr_init_instance(&self->super, "nullv_setattr"); - self->super.eval = _nullv_setattr_eval; - self->super.optimize = _optimize; - self->super.init = _init; - self->super.deinit = _deinit; - self->super.free_fn = _free; - self->object = object; - - self->attr = (FilterXObject *) attr_name; - - self->new_value = new_value; - self->super.ignore_falsy_result = TRUE; - return &self->super; -} - /* Takes reference of object and new_value */ FilterXExpr * filterx_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterXExpr *new_value) @@ -231,3 +211,12 @@ filterx_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterXExpr * self->super.ignore_falsy_result = TRUE; return &self->super; } + +FilterXExpr * +filterx_nullv_setattr_new(FilterXExpr *object, FilterXString *attr_name, FilterXExpr *new_value) +{ + FilterXExpr *self = filterx_setattr_new(object, attr_name, new_value); + self->type = "nullv_setattr"; + self->eval = _nullv_setattr_eval; + return self; +} From 1789f6e91126b673922a42f42cd5897a3da28b56 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 28 Dec 2024 20:43:37 +0100 Subject: [PATCH 10/47] filterx: rename _eval() methods to specific names To make it easier to understand stackdumps. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-comparison.c | 6 +++--- lib/filterx/expr-compound.c | 4 ++-- lib/filterx/expr-condition.c | 6 +++--- lib/filterx/expr-done.c | 4 ++-- lib/filterx/expr-drop.c | 4 ++-- lib/filterx/expr-generator.c | 6 +++--- lib/filterx/expr-get-subscript.c | 4 ++-- lib/filterx/expr-getattr.c | 4 ++-- lib/filterx/expr-isset.c | 4 ++-- lib/filterx/expr-literal.c | 6 +++--- lib/filterx/expr-null-coalesce.c | 4 ++-- lib/filterx/expr-plus.c | 6 +++--- lib/filterx/expr-template.c | 4 ++-- lib/filterx/expr-unset.c | 4 ++-- lib/filterx/expr-variable.c | 6 +++--- lib/filterx/func-flatten.c | 4 ++-- lib/filterx/func-istype.c | 6 +++--- lib/filterx/func-sdata.c | 4 ++-- lib/filterx/func-set-fields.c | 4 ++-- lib/filterx/func-unset-empties.c | 4 ++-- 20 files changed, 47 insertions(+), 47 deletions(-) diff --git a/lib/filterx/expr-comparison.c b/lib/filterx/expr-comparison.c index e94c7d93c1..c912cb5b9a 100644 --- a/lib/filterx/expr-comparison.c +++ b/lib/filterx/expr-comparison.c @@ -198,7 +198,7 @@ _eval_based_on_compare_mode(FilterXExpr *expr, gint compare_mode) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_comparison(FilterXExpr *s) { FilterXComparison *self = (FilterXComparison *) s; @@ -250,7 +250,7 @@ _optimize(FilterXExpr *s) self->literal_rhs = _eval_based_on_compare_mode(self->super.rhs, compare_mode); if (self->literal_lhs && self->literal_rhs) - return filterx_literal_new(_eval(&self->super.super)); + return filterx_literal_new(_eval_comparison(&self->super.super)); return NULL; } @@ -273,7 +273,7 @@ filterx_comparison_new(FilterXExpr *lhs, FilterXExpr *rhs, gint operator) filterx_binary_op_init_instance(&self->super, "comparison", lhs, rhs); self->super.super.optimize = _optimize; - self->super.super.eval = _eval; + self->super.super.eval = _eval_comparison; self->super.super.free_fn = _filterx_comparison_free; self->operator = operator; diff --git a/lib/filterx/expr-compound.c b/lib/filterx/expr-compound.c index d4ca2289af..08b1be494a 100644 --- a/lib/filterx/expr-compound.c +++ b/lib/filterx/expr-compound.c @@ -107,7 +107,7 @@ _eval_exprs(FilterXCompoundExpr *self, FilterXObject **result) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_compound(FilterXExpr *s) { FilterXCompoundExpr *self = (FilterXCompoundExpr *) s; FilterXObject *result = NULL; @@ -232,7 +232,7 @@ filterx_compound_expr_new(gboolean return_value_of_last_expr) FilterXCompoundExpr *self = g_new0(FilterXCompoundExpr, 1); filterx_expr_init_instance(&self->super, "compound"); - self->super.eval = _eval; + self->super.eval = _eval_compound; self->super.optimize = _optimize; self->super.init = _init; self->super.deinit = _deinit; diff --git a/lib/filterx/expr-condition.c b/lib/filterx/expr-condition.c index 3ab0a600da..0bb66ef17b 100644 --- a/lib/filterx/expr-condition.c +++ b/lib/filterx/expr-condition.c @@ -98,7 +98,7 @@ _free(FilterXExpr *s) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_conditional(FilterXExpr *s) { FilterXConditional *self = (FilterXConditional *) s; FilterXObject *condition_value = filterx_expr_eval(self->condition); @@ -213,7 +213,7 @@ filterx_conditional_new(FilterXExpr *condition) { FilterXConditional *self = g_new0(FilterXConditional, 1); filterx_expr_init_instance(&self->super, "conditional"); - self->super.eval = _eval; + self->super.eval = _eval_conditional; self->super.optimize = _optimize; self->super.init = _init; self->super.deinit = _deinit; @@ -227,7 +227,7 @@ FilterXExpr * filterx_conditional_find_tail(FilterXExpr *s) { /* check if this is a FilterXConditional instance */ - if (s->eval != _eval) + if (s->eval != _eval_conditional) return NULL; FilterXConditional *self = (FilterXConditional *) s; diff --git a/lib/filterx/expr-done.c b/lib/filterx/expr-done.c index 4de23e0d9a..a2ff169961 100644 --- a/lib/filterx/expr-done.c +++ b/lib/filterx/expr-done.c @@ -28,7 +28,7 @@ #include "filterx/object-primitive.h" static FilterXObject * -_eval(FilterXExpr *s) +_eval_done(FilterXExpr *s) { FilterXEvalContext *context = filterx_eval_get_context(); context->eval_control_modifier = FXC_DONE; @@ -41,7 +41,7 @@ filterx_expr_done(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); filterx_expr_init_instance(self, "done"); - self->eval = _eval; + self->eval = _eval_done; return self; } diff --git a/lib/filterx/expr-drop.c b/lib/filterx/expr-drop.c index 662fe1c2d2..9e84039632 100644 --- a/lib/filterx/expr-drop.c +++ b/lib/filterx/expr-drop.c @@ -27,7 +27,7 @@ #include "filterx/object-primitive.h" static FilterXObject * -_eval(FilterXExpr *s) +_eval_drop(FilterXExpr *s) { FilterXEvalContext *context = filterx_eval_get_context(); context->eval_control_modifier = FXC_DROP; @@ -40,7 +40,7 @@ filterx_expr_drop_msg(void) { FilterXExpr *self = g_new0(FilterXExpr, 1); filterx_expr_init_instance(self, "drop"); - self->eval = _eval; + self->eval = _eval_drop; return self; } diff --git a/lib/filterx/expr-generator.c b/lib/filterx/expr-generator.c index 6f34599fa0..76a156d3b6 100644 --- a/lib/filterx/expr-generator.c +++ b/lib/filterx/expr-generator.c @@ -34,7 +34,7 @@ filterx_generator_set_fillable(FilterXExpr *s, FilterXExpr *fillable) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_generator(FilterXExpr *s) { FilterXExprGenerator *self = (FilterXExprGenerator *) s; @@ -52,7 +52,7 @@ _eval(FilterXExpr *s) gboolean filterx_expr_is_generator(FilterXExpr *s) { - return s && s->eval == _eval; + return s && s->eval == _eval_generator; } FilterXExpr * @@ -71,7 +71,7 @@ filterx_generator_init_instance(FilterXExpr *s) s->optimize = filterx_generator_optimize_method; s->init = filterx_generator_init_method; s->deinit = filterx_generator_deinit_method; - s->eval = _eval; + s->eval = _eval_generator; s->ignore_falsy_result = TRUE; } diff --git a/lib/filterx/expr-get-subscript.c b/lib/filterx/expr-get-subscript.c index 2251470500..ac2f62e2d5 100644 --- a/lib/filterx/expr-get-subscript.c +++ b/lib/filterx/expr-get-subscript.c @@ -33,7 +33,7 @@ typedef struct _FilterXGetSubscript } FilterXGetSubscript; static FilterXObject * -_eval(FilterXExpr *s) +_eval_get_subscript(FilterXExpr *s) { FilterXGetSubscript *self = (FilterXGetSubscript *) s; FilterXObject *result = NULL; @@ -170,7 +170,7 @@ filterx_get_subscript_new(FilterXExpr *operand, FilterXExpr *key) FilterXGetSubscript *self = g_new0(FilterXGetSubscript, 1); filterx_expr_init_instance(&self->super, "get_subscript"); - self->super.eval = _eval; + self->super.eval = _eval_get_subscript; self->super.is_set = _isset; self->super.unset = _unset; self->super.optimize = _optimize; diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index 08dfc6f9d3..c8c08a5204 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -34,7 +34,7 @@ typedef struct _FilterXGetAttr } FilterXGetAttr; static FilterXObject * -_eval(FilterXExpr *s) +_eval_getattr(FilterXExpr *s) { FilterXGetAttr *self = (FilterXGetAttr *) s; @@ -149,7 +149,7 @@ filterx_getattr_new(FilterXExpr *operand, FilterXString *attr_name) FilterXGetAttr *self = g_new0(FilterXGetAttr, 1); filterx_expr_init_instance(&self->super, "getattr"); - self->super.eval = _eval; + self->super.eval = _eval_getattr; self->super.unset = _unset; self->super.is_set = _isset; self->super.optimize = _optimize; diff --git a/lib/filterx/expr-isset.c b/lib/filterx/expr-isset.c index 65989afadf..f314b2a123 100644 --- a/lib/filterx/expr-isset.c +++ b/lib/filterx/expr-isset.c @@ -25,7 +25,7 @@ #include "filterx/object-primitive.h" static FilterXObject * -_eval(FilterXExpr *s) +_eval_isset(FilterXExpr *s) { FilterXUnaryOp *self = (FilterXUnaryOp *) s; @@ -37,6 +37,6 @@ filterx_isset_new(FilterXExpr *expr) { FilterXUnaryOp *self = g_new0(FilterXUnaryOp, 1); filterx_unary_op_init_instance(self, "isset", expr); - self->super.eval = _eval; + self->super.eval = _eval_isset; return &self->super; } diff --git a/lib/filterx/expr-literal.c b/lib/filterx/expr-literal.c index b01351da78..6e07a96e5c 100644 --- a/lib/filterx/expr-literal.c +++ b/lib/filterx/expr-literal.c @@ -29,7 +29,7 @@ typedef struct _FilterXLiteral } FilterXLiteral; static FilterXObject * -_eval(FilterXExpr *s) +_eval_literal(FilterXExpr *s) { FilterXLiteral *self = (FilterXLiteral *) s; return filterx_object_ref(self->object); @@ -50,7 +50,7 @@ filterx_literal_new(FilterXObject *object) FilterXLiteral *self = g_new0(FilterXLiteral, 1); filterx_expr_init_instance(&self->super, "literal"); - self->super.eval = _eval; + self->super.eval = _eval_literal; self->super.free_fn = _free; self->object = object; return &self->super; @@ -59,5 +59,5 @@ filterx_literal_new(FilterXObject *object) gboolean filterx_expr_is_literal(FilterXExpr *expr) { - return expr->eval == _eval; + return expr->eval == _eval_literal; } diff --git a/lib/filterx/expr-null-coalesce.c b/lib/filterx/expr-null-coalesce.c index 2498ca42f0..e109906494 100644 --- a/lib/filterx/expr-null-coalesce.c +++ b/lib/filterx/expr-null-coalesce.c @@ -37,7 +37,7 @@ struct _FilterXNullCoalesce }; static FilterXObject * -_eval(FilterXExpr *s) +_eval_null_coalesce(FilterXExpr *s) { FilterXNullCoalesce *self = (FilterXNullCoalesce *) s; @@ -86,7 +86,7 @@ filterx_null_coalesce_new(FilterXExpr *lhs, FilterXExpr *rhs) { FilterXNullCoalesce *self = g_new0(FilterXNullCoalesce, 1); filterx_binary_op_init_instance(&self->super, "null_coalesce", lhs, rhs); - self->super.super.eval = _eval; + self->super.super.eval = _eval_null_coalesce; self->super.super.optimize = _optimize; return &self->super.super; } diff --git a/lib/filterx/expr-plus.c b/lib/filterx/expr-plus.c index f31d7f1813..9969d05e9b 100644 --- a/lib/filterx/expr-plus.c +++ b/lib/filterx/expr-plus.c @@ -34,7 +34,7 @@ typedef struct FilterXOperatorPlus } FilterXOperatorPlus; static FilterXObject * -_eval(FilterXExpr *s) +_eval_plus(FilterXExpr *s) { FilterXOperatorPlus *self = (FilterXOperatorPlus *) s; @@ -72,7 +72,7 @@ _optimize(FilterXExpr *s) self->literal_rhs = filterx_expr_eval(self->super.rhs); if (self->literal_lhs && self->literal_rhs) - return filterx_literal_new(_eval(&self->super.super)); + return filterx_literal_new(_eval_plus(&self->super.super)); return NULL; } @@ -92,7 +92,7 @@ filterx_operator_plus_new(FilterXExpr *lhs, FilterXExpr *rhs) FilterXOperatorPlus *self = g_new0(FilterXOperatorPlus, 1); filterx_binary_op_init_instance(&self->super, "plus", lhs, rhs); self->super.super.optimize = _optimize; - self->super.super.eval = _eval; + self->super.super.eval = _eval_plus; self->super.super.free_fn = _filterx_operator_plus_free; return &self->super.super; diff --git a/lib/filterx/expr-template.c b/lib/filterx/expr-template.c index 17653e869b..8e23d15fc7 100644 --- a/lib/filterx/expr-template.c +++ b/lib/filterx/expr-template.c @@ -35,7 +35,7 @@ typedef struct _FilterXTemplate } FilterXTemplate; static FilterXObject * -_eval(FilterXExpr *s) +_eval_template(FilterXExpr *s) { FilterXTemplate *self = (FilterXTemplate *) s; FilterXEvalContext *context = filterx_eval_get_context(); @@ -102,7 +102,7 @@ filterx_template_new(LogTemplate *template) filterx_expr_init_instance(&self->super, "template"); self->super.init = _template_init; self->super.deinit = _template_deinit; - self->super.eval = _eval; + self->super.eval = _eval_template; self->super.free_fn = _free; self->template = template; return &self->super; diff --git a/lib/filterx/expr-unset.c b/lib/filterx/expr-unset.c index b536fcb027..60787d85e6 100644 --- a/lib/filterx/expr-unset.c +++ b/lib/filterx/expr-unset.c @@ -32,7 +32,7 @@ typedef struct FilterXExprUnset_ } FilterXExprUnset; static FilterXObject * -_eval(FilterXExpr *s) +_eval_unset(FilterXExpr *s) { FilterXExprUnset *self = (FilterXExprUnset *) s; @@ -110,7 +110,7 @@ filterx_function_unset_new(FilterXFunctionArgs *args, GError **error) FilterXExprUnset *self = g_new0(FilterXExprUnset, 1); filterx_function_init_instance(&self->super, "unset"); - self->super.super.eval = _eval; + self->super.super.eval = _eval_unset; self->super.super.optimize = _optimize; self->super.super.init = _init; self->super.super.deinit = _deinit; diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 577d66600f..5aa7c95d99 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -65,7 +65,7 @@ _whiteout_variable(FilterXVariableExpr *self, FilterXEvalContext *context) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_variable(FilterXExpr *s) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; FilterXEvalContext *context = filterx_eval_get_context(); @@ -212,7 +212,7 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) self->super.free_fn = _free; self->super.init = _init; self->super.deinit = _deinit; - self->super.eval = _eval; + self->super.eval = _eval_variable; self->super._update_repr = _update_repr; self->super.assign = _assign; self->super.is_set = _isset; @@ -243,6 +243,6 @@ filterx_variable_expr_declare(FilterXExpr *s) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; - g_assert(s->eval == _eval); + g_assert(s->eval == _eval_variable); self->declared = TRUE; } diff --git a/lib/filterx/func-flatten.c b/lib/filterx/func-flatten.c index b5d42e9948..0071200f8f 100644 --- a/lib/filterx/func-flatten.c +++ b/lib/filterx/func-flatten.c @@ -184,7 +184,7 @@ _flatten(FilterXFunctionFlatten *self, FilterXObject *dict) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_fx_flatten(FilterXExpr *s) { FilterXFunctionFlatten *self = (FilterXFunctionFlatten *) s; @@ -291,7 +291,7 @@ filterx_function_flatten_new(FilterXFunctionArgs *args, GError **error) { FilterXFunctionFlatten *self = g_new0(FilterXFunctionFlatten, 1); filterx_function_init_instance(&self->super, "flatten"); - self->super.super.eval = _eval; + self->super.super.eval = _eval_fx_flatten; self->super.super.optimize = _optimize; self->super.super.init = _init; self->super.super.deinit = _deinit; diff --git a/lib/filterx/func-istype.c b/lib/filterx/func-istype.c index 7def2dd501..8e93a4d514 100644 --- a/lib/filterx/func-istype.c +++ b/lib/filterx/func-istype.c @@ -41,7 +41,7 @@ typedef struct FilterXFunctionIsType_ } FilterXFunctionIsType; static FilterXObject * -_eval(FilterXExpr *s) +_eval_fx_istype(FilterXExpr *s) { FilterXFunctionIsType *self = (FilterXFunctionIsType *) s; @@ -68,7 +68,7 @@ _optimize(FilterXExpr *s) if (filterx_expr_is_literal(self->object_expr)) { - FilterXObject *optimized_object = _eval(s); + FilterXObject *optimized_object = _eval_fx_istype(s); g_assert(optimized_object); return filterx_literal_new(optimized_object); } @@ -165,7 +165,7 @@ filterx_function_istype_new(FilterXFunctionArgs *args, GError **error) { FilterXFunctionIsType *self = g_new0(FilterXFunctionIsType, 1); filterx_function_init_instance(&self->super, "istype"); - self->super.super.eval = _eval; + self->super.super.eval = _eval_fx_istype; self->super.super.optimize = _optimize; self->super.super.init = _init; self->super.super.deinit = _deinit; diff --git a/lib/filterx/func-sdata.c b/lib/filterx/func-sdata.c index 5505545bbf..bda295353c 100644 --- a/lib/filterx/func-sdata.c +++ b/lib/filterx/func-sdata.c @@ -67,7 +67,7 @@ _extract_args(FilterXFunctionIsSdataFromEnteprise *self, FilterXFunctionArgs *ar } static FilterXObject * -_eval(FilterXExpr *s) +_eval_fx_is_sdata_from(FilterXExpr *s) { FilterXFunctionIsSdataFromEnteprise *self = (FilterXFunctionIsSdataFromEnteprise *) s; @@ -105,7 +105,7 @@ filterx_function_is_sdata_from_enterprise_new(FilterXFunctionArgs *args, GError if (!_extract_args(self, args, error) || !filterx_function_args_check(args, error)) goto error; - self->super.super.eval = _eval; + self->super.super.eval = _eval_fx_is_sdata_from; self->super.super.free_fn = _free; filterx_function_args_free(args); return &self->super.super; diff --git a/lib/filterx/func-set-fields.c b/lib/filterx/func-set-fields.c index 3ab5ccde66..d272352b63 100644 --- a/lib/filterx/func-set-fields.c +++ b/lib/filterx/func-set-fields.c @@ -206,7 +206,7 @@ _process_field(Field *field, FilterXObject *dict) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_fx_set_fields(FilterXExpr *s) { FilterXFunctionSetFields *self = (FilterXFunctionSetFields *) s; @@ -495,7 +495,7 @@ filterx_function_set_fields_new(FilterXFunctionArgs *args, GError **error) FilterXFunctionSetFields *self = g_new0(FilterXFunctionSetFields, 1); filterx_function_init_instance(&self->super, "set_fields"); - self->super.super.eval = _eval; + self->super.super.eval = _eval_fx_set_fields; self->super.super.optimize = _optimize; self->super.super.init = _init; self->super.super.deinit = _deinit; diff --git a/lib/filterx/func-unset-empties.c b/lib/filterx/func-unset-empties.c index e3ee3edbce..200ba12546 100644 --- a/lib/filterx/func-unset-empties.c +++ b/lib/filterx/func-unset-empties.c @@ -224,7 +224,7 @@ _process_list(FilterXFunctionUnsetEmpties *self, FilterXObject *obj) } static FilterXObject * -_eval(FilterXExpr *s) +_eval_fx_unset_empties(FilterXExpr *s) { FilterXFunctionUnsetEmpties *self = (FilterXFunctionUnsetEmpties *) s; @@ -514,7 +514,7 @@ filterx_function_unset_empties_new(FilterXFunctionArgs *args, GError **error) { FilterXFunctionUnsetEmpties *self = g_new0(FilterXFunctionUnsetEmpties, 1); filterx_function_init_instance(&self->super, "unset_empties"); - self->super.super.eval = _eval; + self->super.super.eval = _eval_fx_unset_empties; self->super.super.optimize = _optimize; self->super.super.init = _init; self->super.super.deinit = _deinit; From a043f0433371eb20b4cb98fd244ef942375a8a80 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 23:24:11 +0100 Subject: [PATCH 11/47] filterx/filterx-scope: add filterx_variable_handle_is_message_tied() in addition to is_floating Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 2 +- lib/filterx/filterx-scope.c | 4 ++-- lib/filterx/filterx-variable.h | 12 ++++++++++++ lib/filterx/func-vars.c | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 4c962722d0..d4bdc23211 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -82,7 +82,7 @@ _eval(FilterXExpr *s) return value; } - if (!filterx_variable_handle_is_floating(self->handle)) + if (filterx_variable_handle_is_message_tied(self->handle)) { FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msgs[0]); if(!msg_ref) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index ae673e7622..51223caa66 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -158,7 +158,7 @@ filterx_scope_register_variable(FilterXScope *self, /* the scope needs to be synced with the message if it holds a * message-tied variable (e.g. $MSG) */ - if (!filterx_variable_handle_is_floating(handle)) + if (filterx_variable_handle_is_message_tied(handle)) self->syncable = TRUE; return v; } @@ -297,7 +297,7 @@ filterx_scope_clone(FilterXScope *other) { FilterXVariable *v = &g_array_index(other->variables, FilterXVariable, src_index); - if (filterx_variable_is_declared(v) || !filterx_variable_is_floating(v)) + if (filterx_variable_is_declared(v) || filterx_variable_is_message_tied(v)) { g_array_append_val(self->variables, *v); FilterXVariable *v_clone = &g_array_index(self->variables, FilterXVariable, dst_index); diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 7b20744232..9913c8152a 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -68,6 +68,12 @@ filterx_variable_handle_is_floating(FilterXVariableHandle handle) return !!(handle & FILTERX_HANDLE_FLOATING_BIT); } +static inline gboolean +filterx_variable_handle_is_message_tied(FilterXVariableHandle handle) +{ + return !filterx_variable_handle_is_floating(handle); +} + static inline NVHandle filterx_variable_handle_to_nv_handle(FilterXVariableHandle handle) { @@ -80,6 +86,12 @@ filterx_variable_is_floating(FilterXVariable *v) return filterx_variable_handle_is_floating(v->handle); } +static inline gboolean +filterx_variable_is_message_tied(FilterXVariable *v) +{ + return filterx_variable_handle_is_message_tied(v->handle); +} + static inline NVHandle filterx_variable_get_nv_handle(FilterXVariable *v) { diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index f78286882c..f760e31b23 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -42,7 +42,7 @@ _add_to_dict(FilterXVariable *variable, gpointer user_data) gssize name_len; const gchar *name_str = filterx_variable_get_name(variable, &name_len); - if (!filterx_variable_is_floating(variable)) + if (filterx_variable_is_message_tied(variable)) { g_string_assign(name_buf, "$"); g_string_append_len(name_buf, name_str, name_len); From 14c61b245e157d6284b3be4a711a5df52d67fc7a Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 16:09:44 +0100 Subject: [PATCH 12/47] filterx/filterx-eval: remove partial support for multiple LogMessage instances We do not really support message contexts (as does the original filter language and templates), so remove that support. This makes it simpler to initialize FilterXEvalContext as well as we do not have to manage the LogMessage array separately. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-template.c | 2 +- lib/filterx/expr-variable.c | 6 +++--- lib/filterx/filterx-eval.c | 7 +++---- lib/filterx/filterx-eval.h | 7 +++---- lib/filterx/filterx-pipe.c | 4 ++-- lib/filterx/func-sdata.c | 6 +++--- libtest/filterx-lib.c | 4 +--- 7 files changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/filterx/expr-template.c b/lib/filterx/expr-template.c index f94716ad34..27bce3001d 100644 --- a/lib/filterx/expr-template.c +++ b/lib/filterx/expr-template.c @@ -46,7 +46,7 @@ _eval(FilterXExpr *s) /* FIXME: we could go directly to filterx_string_new() here to avoid a round trip in FilterXMessageValue */ /* FIXME/2: let's make this handle literal and trivial templates */ - log_template_format_value_and_type_with_context(self->template, context->msgs, context->num_msg, + log_template_format_value_and_type_with_context(self->template, &context->msg, 1, &context->template_eval_options, value, &t); /* NOTE: we borrow value->str here which is stored in a scratch buffer diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index d4bdc23211..670776d672 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -84,7 +84,7 @@ _eval(FilterXExpr *s) if (filterx_variable_handle_is_message_tied(self->handle)) { - FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msgs[0]); + FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msg); if(!msg_ref) return NULL; filterx_scope_register_variable(context->scope, self->handle, msg_ref); @@ -142,7 +142,7 @@ _isset(FilterXExpr *s) return filterx_variable_is_set(variable); FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; return log_msg_is_value_set(msg, self->handle); } @@ -159,7 +159,7 @@ _unset(FilterXExpr *s) return TRUE; } - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; if (log_msg_is_value_set(msg, self->handle)) _whiteout_variable(self, context); diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c index 9d510a9935..eb815b4c91 100644 --- a/lib/filterx/filterx-eval.c +++ b/lib/filterx/filterx-eval.c @@ -119,10 +119,8 @@ filterx_format_last_error_location(void) } FilterXEvalResult -filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr, LogMessage *msg) +filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr) { - context->msgs = &msg; - context->num_msg = 1; FilterXEvalResult result = FXE_FAILURE; FilterXObject *res = filterx_expr_eval(expr); @@ -147,7 +145,7 @@ filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr, LogMessage *ms } void -filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context) +filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, LogMessage *msg) { FilterXScope *scope; @@ -158,6 +156,7 @@ filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previ filterx_scope_make_writable(&scope); memset(context, 0, sizeof(*context)); + context->msg = msg; context->template_eval_options = DEFAULT_TEMPLATE_EVAL_OPTIONS; context->scope = scope; diff --git a/lib/filterx/filterx-eval.h b/lib/filterx/filterx-eval.h index 8c995cbf06..34d3a3f8a3 100644 --- a/lib/filterx/filterx-eval.h +++ b/lib/filterx/filterx-eval.h @@ -47,8 +47,7 @@ typedef enum _FilterXEvalControl typedef struct _FilterXEvalContext FilterXEvalContext; struct _FilterXEvalContext { - LogMessage **msgs; - gint num_msg; + LogMessage *msg; FilterXScope *scope; FilterXError error; LogTemplateEvalOptions template_eval_options; @@ -62,14 +61,14 @@ FilterXScope *filterx_eval_get_scope(void); void filterx_eval_push_error(const gchar *message, FilterXExpr *expr, FilterXObject *object); void filterx_eval_push_error_info(const gchar *message, FilterXExpr *expr, gchar *info, gboolean free_info); void filterx_eval_set_context(FilterXEvalContext *context); -FilterXEvalResult filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr, LogMessage *msg); +FilterXEvalResult filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr); const gchar *filterx_eval_get_last_error(void); EVTTAG *filterx_format_last_error(void); EVTTAG *filterx_format_last_error_location(void); void filterx_eval_clear_errors(void); EVTTAG *filterx_format_eval_result(FilterXEvalResult result); -void filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context); +void filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, LogMessage *msg); void filterx_eval_deinit_context(FilterXEvalContext *context); static inline void diff --git a/lib/filterx/filterx-pipe.c b/lib/filterx/filterx-pipe.c index 20c7dad909..72c3fb4f5a 100644 --- a/lib/filterx/filterx-pipe.c +++ b/lib/filterx/filterx-pipe.c @@ -68,7 +68,7 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o FilterXEvalResult eval_res; path_options = log_path_options_chain(&local_path_options, path_options); - filterx_eval_init_context(&eval_context, path_options->filterx_context); + filterx_eval_init_context(&eval_context, path_options->filterx_context, msg); if (filterx_scope_has_log_msg_changes(eval_context.scope)) filterx_scope_invalidate_log_msg_cache(eval_context.scope); @@ -79,7 +79,7 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o evt_tag_msg_reference(msg)); NVTable *payload = nv_table_ref(msg->payload); - eval_res = filterx_eval_exec(&eval_context, self->block, msg); + eval_res = filterx_eval_exec(&eval_context, self->block); msg_trace("<<<<<< filterx rule evaluation result", filterx_format_eval_result(eval_res), diff --git a/lib/filterx/func-sdata.c b/lib/filterx/func-sdata.c index 5505545bbf..60f5b4abfc 100644 --- a/lib/filterx/func-sdata.c +++ b/lib/filterx/func-sdata.c @@ -73,7 +73,7 @@ _eval(FilterXExpr *s) gboolean contains = FALSE; FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; for (guint8 i = 0; i < msg->num_sdata && !contains; i++) { @@ -127,7 +127,7 @@ filterx_simple_function_has_sdata(FilterXExpr *s, FilterXObject *args[], gsize a } FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; return filterx_boolean_new(msg->num_sdata != 0); } @@ -224,7 +224,7 @@ static gboolean _generate(FilterXExprGenerator *s, FilterXObject *fillable) { FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msgs[0]; + LogMessage *msg = context->msg; const gchar *current_sd_id_start; gsize current_sd_id_len; diff --git a/libtest/filterx-lib.c b/libtest/filterx-lib.c index 2b46c74e9f..d821e48438 100644 --- a/libtest/filterx-lib.c +++ b/libtest/filterx-lib.c @@ -193,9 +193,7 @@ init_libtest_filterx(void) FILTERX_TYPE_NAME(test_list) = FILTERX_TYPE_NAME(json_array); filterx_env.msg = create_sample_message(); - filterx_eval_init_context(&filterx_env.context, NULL); - filterx_env.context.msgs = &filterx_env.msg; - filterx_env.context.num_msg = 1; + filterx_eval_init_context(&filterx_env.context, NULL, filterx_env.msg); } From 32cd7af86e0892579d454d10cf7815a1fe19665f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 09:35:10 +0100 Subject: [PATCH 13/47] filterx/expr-variable: make template macro based variables read only Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 670776d672..ee76cfaea0 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -113,6 +113,12 @@ _assign(FilterXExpr *s, FilterXObject *new_value) FilterXScope *scope = filterx_eval_get_scope(); FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); + if (self->handle_is_macro) + { + filterx_eval_push_error("Macro based variable cannot be changed", &self->super, self->variable_name); + return FALSE; + } + if (!variable) { /* NOTE: we pass NULL as initial_value to make sure the new variable @@ -150,6 +156,13 @@ static gboolean _unset(FilterXExpr *s) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; + + if (self->handle_is_macro) + { + filterx_eval_push_error("Macro based variable cannot be changed", &self->super, self->variable_name); + return FALSE; + } + FilterXEvalContext *context = filterx_eval_get_context(); FilterXVariable *variable = filterx_scope_lookup_variable(context->scope, self->handle); From 01720425f872d8875d7c79add565d1c551f9ae82 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:15:13 +0100 Subject: [PATCH 14/47] filterx/filterx-variable: move FilterXVariableHandle related functions to the front Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-variable.h | 47 +++++++++++++++++----------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 9913c8152a..65346770b8 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -29,25 +29,6 @@ #include "syslog-ng.h" #include "filterx-object.h" -#define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) - -typedef guint32 FilterXVariableHandle; - -typedef struct _FilterXVariable -{ - /* the MSB indicates that the variable is a floating one */ - FilterXVariableHandle handle; - /* - * assigned -- Indicates that the variable was assigned to a new value - * - * declared -- this variable is declared (e.g. retained for the entire input pipeline) - */ - guint32 assigned:1, - declared:1, - generation:20; - FilterXObject *value; -} FilterXVariable; - typedef enum { FX_VAR_MESSAGE, @@ -55,10 +36,9 @@ typedef enum FX_VAR_DECLARED, } FilterXVariableType; +#define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) -void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, - FilterXObject *initial_value, guint32 generation); -void filterx_variable_free(FilterXVariable *v); +typedef guint32 FilterXVariableHandle; #define FILTERX_HANDLE_FLOATING_BIT (1UL << 31) @@ -80,6 +60,27 @@ filterx_variable_handle_to_nv_handle(FilterXVariableHandle handle) return handle & ~FILTERX_HANDLE_FLOATING_BIT; } +FilterXVariableHandle filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type); + +typedef struct _FilterXVariable +{ + /* the MSB indicates that the variable is a floating one */ + FilterXVariableHandle handle; + /* + * assigned -- Indicates that the variable was assigned to a new value + * + * declared -- this variable is declared (e.g. retained for the entire input pipeline) + */ + guint32 assigned:1, + declared:1, + generation:20; + FilterXObject *value; +} FilterXVariable; + +void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, + FilterXObject *initial_value, guint32 generation); +void filterx_variable_free(FilterXVariable *v); + static inline gboolean filterx_variable_is_floating(FilterXVariable *v) { @@ -166,6 +167,4 @@ filterx_variable_set_declared(FilterXVariable *v, gboolean declared) v->declared = declared; } -FilterXVariableHandle filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type); - #endif From 24aed204631c87a067f1edb04b128051d87ad84b Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:36:54 +0100 Subject: [PATCH 15/47] filterx/filterx-variable: rename filterx_variable_free to _clear() It does not really free @self, so it only clears up allocations within an existing instance, these are usually called *_clear() functions. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 4 ++-- lib/filterx/filterx-variable.c | 2 +- lib/filterx/filterx-variable.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 51223caa66..f85d839b78 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -284,7 +284,7 @@ filterx_scope_new(void) g_atomic_counter_set(&self->ref_cnt, 1); self->variables = g_array_sized_new(FALSE, TRUE, sizeof(FilterXVariable), 16); - g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_free); + g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_clear); return self; } @@ -391,7 +391,7 @@ filterx_scope_invalidate_log_msg_cache(FilterXScope *self) if (!filterx_variable_is_floating(v) && self->syncable) { /* skip this variable */ - filterx_variable_free(v); + filterx_variable_clear(v); } else { diff --git a/lib/filterx/filterx-variable.c b/lib/filterx/filterx-variable.c index cd14418d61..95a10e08d2 100644 --- a/lib/filterx/filterx-variable.c +++ b/lib/filterx/filterx-variable.c @@ -39,7 +39,7 @@ filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type) } void -filterx_variable_free(FilterXVariable *v) +filterx_variable_clear(FilterXVariable *v) { filterx_object_unref(v->value); } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 65346770b8..2cd498b52a 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -79,7 +79,7 @@ typedef struct _FilterXVariable void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, FilterXObject *initial_value, guint32 generation); -void filterx_variable_free(FilterXVariable *v); +void filterx_variable_clear(FilterXVariable *v); static inline gboolean filterx_variable_is_floating(FilterXVariable *v) From ae4172537b2f6c8811bfd78a9efd493239147e4c Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:27:09 +0100 Subject: [PATCH 16/47] filterx/filterx-variable: introduce "variable_type" at the variable layer Previously the type enum was only used during the initialization of FilterXExprVariable which was then translated to a set of booleans at the variable layer. Also clean up naming a bit. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 27 ++++++++++++++------------- lib/filterx/filterx-scope.c | 21 ++++----------------- lib/filterx/filterx-scope.h | 4 +--- lib/filterx/filterx-variable.c | 13 ++++++++----- lib/filterx/filterx-variable.h | 21 +++++++++------------ lib/filterx/func-vars.c | 11 ++++------- 6 files changed, 40 insertions(+), 57 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index ee76cfaea0..2eede2f4eb 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -34,9 +34,10 @@ typedef struct _FilterXVariableExpr { FilterXExpr super; - FilterXObject *variable_name; + FilterXVariableType variable_type; NVHandle handle; - guint32 declared:1, handle_is_macro:1; + FilterXObject *variable_name; + guint32 handle_is_macro:1; } FilterXVariableExpr; static FilterXObject * @@ -61,7 +62,7 @@ _pull_variable_from_message(FilterXVariableExpr *self, FilterXEvalContext *conte static void _whiteout_variable(FilterXVariableExpr *self, FilterXEvalContext *context) { - filterx_scope_register_variable(context->scope, self->handle, NULL); + filterx_scope_register_variable(context->scope, FX_VAR_MESSAGE_TIED, self->handle, NULL); } static FilterXObject * @@ -87,7 +88,7 @@ _eval(FilterXExpr *s) FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msg); if(!msg_ref) return NULL; - filterx_scope_register_variable(context->scope, self->handle, msg_ref); + filterx_scope_register_variable(context->scope, FX_VAR_MESSAGE_TIED, self->handle, msg_ref); return msg_ref; } @@ -124,10 +125,7 @@ _assign(FilterXExpr *s, FilterXObject *new_value) /* NOTE: we pass NULL as initial_value to make sure the new variable * is considered changed due to the assignment */ - if (self->declared) - variable = filterx_scope_register_declared_variable(scope, self->handle, NULL); - else - variable = filterx_scope_register_variable(scope, self->handle, NULL); + variable = filterx_scope_register_variable(scope, self->variable_type, self->handle, NULL); } /* this only clones mutable objects */ @@ -217,7 +215,7 @@ _deinit(FilterXExpr *s, GlobalConfig *cfg) } static FilterXExpr * -filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) +filterx_variable_expr_new(FilterXString *name, FilterXVariableType variable_type) { FilterXVariableExpr *self = g_new0(FilterXVariableExpr, 1); @@ -231,9 +229,10 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) self->super.is_set = _isset; self->super.unset = _unset; + self->variable_type = variable_type; self->variable_name = (FilterXObject *) name; - self->handle = filterx_map_varname_to_handle(filterx_string_get_value_ref(self->variable_name, NULL), type); - if (type == FX_VAR_MESSAGE) + self->handle = filterx_map_varname_to_handle(filterx_string_get_value_ref(self->variable_name, NULL), variable_type); + if (variable_type == FX_VAR_MESSAGE_TIED) self->handle_is_macro = log_msg_is_handle_macro(filterx_variable_handle_to_nv_handle(self->handle)); return &self->super; @@ -242,7 +241,7 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type) FilterXExpr * filterx_msg_variable_expr_new(FilterXString *name) { - return filterx_variable_expr_new(name, FX_VAR_MESSAGE); + return filterx_variable_expr_new(name, FX_VAR_MESSAGE_TIED); } FilterXExpr * @@ -257,5 +256,7 @@ filterx_variable_expr_declare(FilterXExpr *s) FilterXVariableExpr *self = (FilterXVariableExpr *) s; g_assert(s->eval == _eval); - self->declared = TRUE; + /* we can only declare a floating variable */ + g_assert(self->variable_type == FX_VAR_FLOATING); + self->variable_type = FX_VAR_DECLARED_FLOATING; } diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index f85d839b78..d742ae1998 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -115,6 +115,7 @@ filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) static FilterXVariable * _register_variable(FilterXScope *self, + FilterXVariableType variable_type, FilterXVariableHandle handle, FilterXObject *initial_value) { @@ -142,7 +143,7 @@ _register_variable(FilterXScope *self, FilterXVariable v; - filterx_variable_init_instance(&v, handle, initial_value, self->generation); + filterx_variable_init_instance(&v, variable_type, handle, initial_value, self->generation); g_array_insert_val(self->variables, v_index, v); return &g_array_index(self->variables, FilterXVariable, v_index); @@ -150,11 +151,11 @@ _register_variable(FilterXScope *self, FilterXVariable * filterx_scope_register_variable(FilterXScope *self, + FilterXVariableType variable_type, FilterXVariableHandle handle, FilterXObject *initial_value) { - FilterXVariable *v = _register_variable(self, handle, initial_value); - filterx_variable_set_declared(v, FALSE); + FilterXVariable *v = _register_variable(self, variable_type, handle, initial_value); /* the scope needs to be synced with the message if it holds a * message-tied variable (e.g. $MSG) */ @@ -163,20 +164,6 @@ filterx_scope_register_variable(FilterXScope *self, return v; } -FilterXVariable * -filterx_scope_register_declared_variable(FilterXScope *self, - FilterXVariableHandle handle, - FilterXObject *initial_value) - -{ - g_assert(filterx_variable_handle_is_floating(handle)); - - FilterXVariable *v = _register_variable(self, handle, initial_value); - filterx_variable_set_declared(v, TRUE); - - return v; -} - gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data) { diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 89f12c3664..2e996d2020 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -52,11 +52,9 @@ void filterx_scope_sync(FilterXScope *self, LogMessage *msg); FilterXVariable *filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle); FilterXVariable *filterx_scope_register_variable(FilterXScope *self, + FilterXVariableType variable_type, FilterXVariableHandle handle, FilterXObject *initial_value); -FilterXVariable *filterx_scope_register_declared_variable(FilterXScope *self, - FilterXVariableHandle handle, - FilterXObject *initial_value); gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data); void filterx_scope_invalidate_log_msg_cache(FilterXScope *self); diff --git a/lib/filterx/filterx-variable.c b/lib/filterx/filterx-variable.c index 95a10e08d2..3b5b726e2d 100644 --- a/lib/filterx/filterx-variable.c +++ b/lib/filterx/filterx-variable.c @@ -28,12 +28,12 @@ FilterXVariableHandle filterx_map_varname_to_handle(const gchar *name, FilterXVariableType type) { - if (type == FX_VAR_MESSAGE) + if (type == FX_VAR_MESSAGE_TIED) name++; NVHandle nv_handle = log_msg_get_value_handle(name); - if (type == FX_VAR_MESSAGE) + if (type == FX_VAR_MESSAGE_TIED) return (FilterXVariableHandle) nv_handle; return (FilterXVariableHandle) nv_handle | FILTERX_HANDLE_FLOATING_BIT; } @@ -45,12 +45,15 @@ filterx_variable_clear(FilterXVariable *v) } void -filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, - FilterXObject *initial_value, guint32 generation) +filterx_variable_init_instance(FilterXVariable *v, + FilterXVariableType variable_type, + FilterXVariableHandle handle, + FilterXObject *initial_value, + guint32 generation) { v->handle = handle; + v->variable_type = variable_type; v->assigned = FALSE; - v->declared = FALSE; v->generation = generation; v->value = filterx_object_ref(initial_value); } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 2cd498b52a..447774c1c1 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -31,9 +31,9 @@ typedef enum { - FX_VAR_MESSAGE, + FX_VAR_MESSAGE_TIED, FX_VAR_FLOATING, - FX_VAR_DECLARED, + FX_VAR_DECLARED_FLOATING, } FilterXVariableType; #define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) @@ -72,13 +72,16 @@ typedef struct _FilterXVariable * declared -- this variable is declared (e.g. retained for the entire input pipeline) */ guint32 assigned:1, - declared:1, + variable_type:2, generation:20; FilterXObject *value; } FilterXVariable; -void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableHandle handle, - FilterXObject *initial_value, guint32 generation); +void filterx_variable_init_instance(FilterXVariable *v, + FilterXVariableType variable_type, + FilterXVariableHandle handle, + FilterXObject *initial_value, + guint32 generation); void filterx_variable_clear(FilterXVariable *v); static inline gboolean @@ -134,7 +137,7 @@ filterx_variable_is_set(FilterXVariable *v) static inline gboolean filterx_variable_is_declared(FilterXVariable *v) { - return v->declared; + return v->variable_type == FX_VAR_DECLARED_FLOATING; } static inline gboolean @@ -161,10 +164,4 @@ filterx_variable_unassign(FilterXVariable *v) v->assigned = FALSE; } -static inline void -filterx_variable_set_declared(FilterXVariable *v, gboolean declared) -{ - v->declared = declared; -} - #endif diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index f760e31b23..6c24ceed5e 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -111,14 +111,11 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) return FALSE; } - gboolean is_floating = key_str[0] != '$'; - FilterXVariableHandle handle = filterx_map_varname_to_handle(key_str, is_floating ? FX_VAR_FLOATING : FX_VAR_MESSAGE); + FilterXVariableType variable_type = (key_str[0] == '$') ? FX_VAR_MESSAGE_TIED : FX_VAR_DECLARED_FLOATING; + FilterXVariableHandle handle = filterx_map_varname_to_handle(key_str, variable_type); FilterXVariable *variable = NULL; - if (is_floating) - variable = filterx_scope_register_declared_variable(scope, handle, NULL); - else - variable = filterx_scope_register_variable(scope, handle, NULL); + variable = filterx_scope_register_variable(scope, variable_type, handle, NULL); FilterXObject *cloned_value = filterx_object_clone(value); filterx_variable_set_value(variable, cloned_value); @@ -143,7 +140,7 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) evt_tag_str("key", key_str), evt_tag_str("value", repr->str), evt_tag_str("type", value->type->name), - evt_tag_str("variable_type", is_floating ? "declared" : "message")); + evt_tag_int("variable_type", variable_type)); } return TRUE; From 8947d6d1f46c969a53742fc1763765fcb0125192 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 15:24:03 +0100 Subject: [PATCH 17/47] logmsg: add generation counter Signed-off-by: Balazs Scheidler --- lib/logmsg/logmsg.c | 5 +++++ lib/logmsg/logmsg.h | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/lib/logmsg/logmsg.c b/lib/logmsg/logmsg.c index d544ac3a77..9eab675955 100644 --- a/lib/logmsg/logmsg.c +++ b/lib/logmsg/logmsg.c @@ -593,6 +593,7 @@ log_msg_set_value_with_type(LogMessage *self, NVHandle handle, if (value_len < 0) value_len = strlen(value); + self->generation++; if (_log_name_value_updates(self)) { msg_trace("Setting value", @@ -651,6 +652,7 @@ log_msg_unset_value(LogMessage *self, NVHandle handle) { g_assert(!log_msg_is_write_protected(self)); + self->generation++; if (_log_name_value_updates(self)) { msg_trace("Unsetting value", @@ -712,6 +714,7 @@ log_msg_set_value_indirect_with_type(LogMessage *self, NVHandle handle, name_len = 0; name = log_msg_get_value_name(handle, &name_len); + self->generation++; if (_log_name_value_updates(self)) { msg_trace("Setting indirect value", @@ -951,6 +954,7 @@ log_msg_set_tag_by_id_onoff(LogMessage *self, LogTagId id, gboolean on) g_assert(!log_msg_is_write_protected(self)); + self->generation++; msg_trace("Setting tag", evt_tag_str("name", log_tags_get_by_id(id)), evt_tag_int("value", on), @@ -1358,6 +1362,7 @@ log_msg_clear(LogMessage *self) { g_assert(!log_msg_is_write_protected(self)); + self->generation++; if(log_msg_chk_flag(self, LF_STATE_OWN_PAYLOAD)) nv_table_unref(self->payload); self->payload = nv_table_new(LM_V_MAX, 16, 256); diff --git a/lib/logmsg/logmsg.h b/lib/logmsg/logmsg.h index 036dc9d81a..5d9d588a12 100644 --- a/lib/logmsg/logmsg.h +++ b/lib/logmsg/logmsg.h @@ -260,6 +260,10 @@ struct _LogMessage gulong *tags; NVHandle *sdata; + /* this member is incremented for any write operation and it can also + * overflow, so only track it for changes and assume that 2^16 operations + * would suffice between two checks */ + guint16 generation; guint16 pri; guint8 initial_parse:1, recursed:1, From 0d0566d3f7d9b8cb54858205fb41faa30417f80b Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 4 Jan 2025 09:09:51 +0100 Subject: [PATCH 18/47] filterx/filterx-scope: keep track of the maximum number of variables Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index ae673e7622..6a2e0085c7 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -30,6 +30,8 @@ struct _FilterXScope guint32 generation:20, write_protected, dirty, syncable, log_msg_has_changes; }; +volatile gint filterx_scope_variables_max = 16; + static gboolean _lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariable **v_slot) { @@ -283,7 +285,7 @@ filterx_scope_new(void) FilterXScope *self = g_new0(FilterXScope, 1); g_atomic_counter_set(&self->ref_cnt, 1); - self->variables = g_array_sized_new(FALSE, TRUE, sizeof(FilterXVariable), 16); + self->variables = g_array_sized_new(FALSE, TRUE, sizeof(FilterXVariable), filterx_scope_variables_max); g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_free); return self; } @@ -353,6 +355,11 @@ filterx_scope_make_writable(FilterXScope **pself) static void _free(FilterXScope *self) { + /* NOTE: update the number of inlined variable allocations */ + gint variables_max = filterx_scope_variables_max; + if (variables_max < self->variables->len) + filterx_scope_variables_max = self->variables->len; + g_array_free(self->variables, TRUE); g_free(self); } From fd3ecd306ea3b2f54d943e3269beb28aa355e781 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 16:10:01 +0100 Subject: [PATCH 19/47] filterx: do not consider unmarshaling a change in value If we pull in a variable from the message and we need to unmarshal it (e.g. turn FilterXMessageValue to a more specific type like FilterXString or FilterXJSON), do not consider that a change of that variable. Changing it in-place, or assignment of a new value should be remain to be a change. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 4 +- lib/filterx/filterx-scope.c | 4 +- lib/filterx/filterx-variable.h | 6 +-- lib/filterx/func-vars.c | 2 +- .../filterx/test_filterx_scope.py | 48 +++++++++++++++++-- 5 files changed, 51 insertions(+), 13 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 2eede2f4eb..c38539c195 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -104,7 +104,7 @@ _update_repr(FilterXExpr *s, FilterXObject *new_repr) FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); g_assert(variable != NULL); - filterx_variable_set_value(variable, new_repr); + filterx_variable_set_value(variable, new_repr, FALSE); } static gboolean @@ -130,7 +130,7 @@ _assign(FilterXExpr *s, FilterXObject *new_value) /* this only clones mutable objects */ new_value = filterx_object_clone(new_value); - filterx_variable_set_value(variable, new_value); + filterx_variable_set_value(variable, new_value, TRUE); filterx_object_unref(new_value); return TRUE; } diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index d742ae1998..95c3658c11 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -130,9 +130,7 @@ _register_variable(FilterXScope *self, * it was a new value */ filterx_variable_set_generation(v_slot, self->generation); - filterx_variable_set_value(v_slot, initial_value); - /* consider this to be unset just as an initial registration is */ - filterx_variable_unassign(v_slot); + filterx_variable_set_value(v_slot, initial_value, FALSE); } return v_slot; } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 447774c1c1..b31afe52ce 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -115,17 +115,17 @@ filterx_variable_get_value(FilterXVariable *v) } static inline void -filterx_variable_set_value(FilterXVariable *v, FilterXObject *new_value) +filterx_variable_set_value(FilterXVariable *v, FilterXObject *new_value, gboolean assignment) { filterx_object_unref(v->value); v->value = filterx_object_ref(new_value); - v->assigned = TRUE; + v->assigned = assignment; } static inline void filterx_variable_unset_value(FilterXVariable *v) { - filterx_variable_set_value(v, NULL); + filterx_variable_set_value(v, NULL, TRUE); } static inline gboolean diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index 6c24ceed5e..d412998db5 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -118,7 +118,7 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) variable = filterx_scope_register_variable(scope, variable_type, handle, NULL); FilterXObject *cloned_value = filterx_object_clone(value); - filterx_variable_set_value(variable, cloned_value); + filterx_variable_set_value(variable, cloned_value, TRUE); filterx_object_unref(cloned_value); if (!variable) diff --git a/tests/light/functional_tests/filterx/test_filterx_scope.py b/tests/light/functional_tests/filterx/test_filterx_scope.py index d62b97207a..78f8d4c6e0 100644 --- a/tests/light/functional_tests/filterx/test_filterx_scope.py +++ b/tests/light/functional_tests/filterx/test_filterx_scope.py @@ -30,10 +30,14 @@ def render_filterx_exprs(expressions): return '\n'.join(f"filterx {{ {expr} }};" for expr in expressions) -def create_config(config, init_exprs, true_exprs=(), false_exprs=(), final_exprs=(), msg="foobar"): - file_true = config.create_file_destination(file_name="dest-true.log", template="'$MSG\n'") - file_false = config.create_file_destination(file_name="dest-false.log", template="'$MSG\n'") - file_final = config.create_file_destination(file_name="dest-final.log", template="'$MSG\n'") +def render_log_exprs(expressions): + return '\n'.join(expressions) + + +def create_config(config, init_exprs, init_log_exprs=(), true_exprs=(), false_exprs=(), final_log_exprs=(), final_exprs=(), msg="foobar", template="'$MSG\n'"): + file_true = config.create_file_destination(file_name="dest-true.log", template=template) + file_false = config.create_file_destination(file_name="dest-false.log", template=template) + file_final = config.create_file_destination(file_name="dest-final.log", template=template) preamble = f""" @version: {config.get_version()} @@ -76,6 +80,7 @@ def create_config(config, init_exprs, true_exprs=(), false_exprs=(), final_exprs log {{ source(genmsg); {render_filterx_exprs(init_exprs)}; + {render_log_exprs(init_log_exprs)} if {{ {render_filterx_exprs(true_exprs)} destination(dest_true); @@ -83,6 +88,7 @@ def create_config(config, init_exprs, true_exprs=(), false_exprs=(), final_exprs {render_filterx_exprs(false_exprs)} destination(dest_false); }}; + {render_log_exprs(final_log_exprs)} {render_filterx_exprs(final_exprs)} destination(dest_final); }}; @@ -182,6 +188,40 @@ def test_message_tied_variables_do_not_propagate_to_parallel_branches(config, sy assert file_false.read_log() == "kecske\n" +def test_message_tied_variables_are_not_considered_changed_just_by_unmarshaling(config, syslog_ng): + (file_true, file_false, file_final) = create_config( + config, init_exprs=[ + """ + # pull up the value from the message into a filterx variable + ${values.json}; + # cause an unmarshal into JSON + ${values.json}.emb_key1; + # $foo is set to the unmarshalled version of ${values.json} + $foo = ${values.json}; + """, + ], init_log_exprs=[ + # trigger a sync + """ + rewrite { + }; + """, + ], + # ${values.json} should retain to have spaces in them, since it was not actually changed, just unmarshalled + # ${foo} is reformatted from the unmarshalled value + # + # NOTE the extra spaces in the assertion below on the $foo part + template="'${values.json} -- $foo\n'", + ) + + syslog_ng.start(config) + + assert file_true.get_stats()["processed"] == 1 + assert "processed" not in file_false.get_stats() + (values_json, foo) = file_true.read_log().strip().split(' -- ') + assert values_json == """{"emb_key1": "emb_key1 value", "emb_key2": "emb_key2 value"}""" + assert foo == """{"emb_key1":"emb_key1 value","emb_key2":"emb_key2 value"}""" + + def test_floating_variables_are_dropped_at_the_end_of_the_scope(config, syslog_ng): (file_true, file_false, _) = create_config( config, [ From 4604d5a143b19775921aa2a58774bb6f563026c9 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 15:04:18 +0100 Subject: [PATCH 20/47] filterx: remove utf8 support from unset_empties() Signed-off-by: Balazs Scheidler --- lib/filterx/func-unset-empties.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/filterx/func-unset-empties.c b/lib/filterx/func-unset-empties.c index e3ee3edbce..d530b057b6 100644 --- a/lib/filterx/func-unset-empties.c +++ b/lib/filterx/func-unset-empties.c @@ -67,7 +67,8 @@ static gboolean _process_list(FilterXFunctionUnsetEmpties *self, FilterXObject * typedef int (*str_cmp_fn)(const char *, const char *); -static gboolean _string_compare(FilterXFunctionUnsetEmpties *self, const gchar *str, str_cmp_fn cmp_fn) +static inline gboolean +_string_compare(FilterXFunctionUnsetEmpties *self, const gchar *str, str_cmp_fn cmp_fn) { guint num_targets = self->targets ? self->targets->len : 0; for (guint i = 0; i < num_targets; i++) @@ -79,11 +80,11 @@ static gboolean _string_compare(FilterXFunctionUnsetEmpties *self, const gchar * return FALSE; } -static gboolean _should_unset_string(FilterXFunctionUnsetEmpties *self, FilterXObject *obj) +static gboolean +_should_unset_string(FilterXFunctionUnsetEmpties *self, FilterXObject *obj) { gsize str_len = 0; const gchar *str = NULL; - gchar *casefold_str = NULL; if (!filterx_object_extract_string_ref(obj, &str, &str_len)) return FALSE; g_assert(str); @@ -91,16 +92,12 @@ static gboolean _should_unset_string(FilterXFunctionUnsetEmpties *self, FilterXO if (check_flag(self->flags, FILTERX_FUNC_UNSET_EMPTIES_FLAG_REPLACE_EMPTY_STRING) && (str_len == 0)) return TRUE; - if (!g_utf8_validate(str, -1, NULL)) - return FALSE; if (check_flag(self->flags, FILTERX_FUNC_UNSET_EMPTIES_FLAG_IGNORECASE)) { - casefold_str = g_utf8_casefold(str, str_len); - gboolean result = _string_compare(self, casefold_str, g_utf8_collate); - g_free(casefold_str); + gboolean result = _string_compare(self, str, strcasecmp); return result; } - return _string_compare(self, str, g_strcmp0); + return _string_compare(self, str, strcmp); } static gboolean @@ -404,18 +401,12 @@ _handle_target_object(FilterXFunctionUnsetEmpties *self, FilterXObject *target, set_flag(&self->flags, FILTERX_FUNC_UNSET_EMPTIES_FLAG_REPLACE_EMPTY_STRING, TRUE); return TRUE; } - if (!g_utf8_validate(str, -1, NULL)) - { - g_set_error(error, FILTERX_FUNCTION_ERROR, FILTERX_FUNCTION_ERROR_CTOR_FAIL, - FILTERX_FUNC_UNSET_EMPTIES_ARG_NAME_TARGETS" strings must be valid utf8 strings! " FILTERX_FUNC_UNSET_EMPTIES_USAGE); - return FALSE; - } if (check_flag(self->flags, FILTERX_FUNC_UNSET_EMPTIES_FLAG_IGNORECASE)) { - g_ptr_array_add(self->targets, g_utf8_casefold(str, len)); + g_ptr_array_add(self->targets, g_strndup(str, len)); return TRUE; } - g_ptr_array_add(self->targets, g_strdup(str)); + g_ptr_array_add(self->targets, g_strndup(str, len)); } else { From e3d5638a97bd87a550ec935aae6c558a9262580c Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 15:07:40 +0100 Subject: [PATCH 21/47] filterx/unset_empties(): set ignorecase to FALSE The others are not expensive, but this one can be. Signed-off-by: Balazs Scheidler --- lib/filterx/func-unset-empties.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/filterx/func-unset-empties.c b/lib/filterx/func-unset-empties.c index d530b057b6..96b7c03d89 100644 --- a/lib/filterx/func-unset-empties.c +++ b/lib/filterx/func-unset-empties.c @@ -511,7 +511,14 @@ filterx_function_unset_empties_new(FilterXFunctionArgs *args, GError **error) self->super.super.deinit = _deinit; self->super.super.free_fn = _free; - reset_flags(&self->flags, ALL_FLAG_SET(FilterXFunctionUnsetEmptiesFlags)); + /* everything is enabled except ignorecase */ + reset_flags(&self->flags, + FLAG_VAL(FILTERX_FUNC_UNSET_EMPTIES_FLAG_RECURSIVE) | + FLAG_VAL(FILTERX_FUNC_UNSET_EMPTIES_FLAG_REPLACE_NULL) | + FLAG_VAL(FILTERX_FUNC_UNSET_EMPTIES_FLAG_REPLACE_EMPTY_STRING) | + FLAG_VAL(FILTERX_FUNC_UNSET_EMPTIES_FLAG_REPLACE_EMPTY_LIST) | + FLAG_VAL(FILTERX_FUNC_UNSET_EMPTIES_FLAG_REPLACE_EMPTY_DICT) + ); if (!_extract_args(self, args, error) || !filterx_function_args_check(args, error)) From 87856f1a683eca763b82617c83a257b20ac7bd13 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 16:05:24 +0100 Subject: [PATCH 22/47] news: added news entry Signed-off-by: Balazs Scheidler --- news/fx-feature-452.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 news/fx-feature-452.md diff --git a/news/fx-feature-452.md b/news/fx-feature-452.md new file mode 100644 index 0000000000..a8c9d5b1d4 --- /dev/null +++ b/news/fx-feature-452.md @@ -0,0 +1,4 @@ +`unset_empties()`: change the default for ignorecase to FALSE, remove utf8 +support. UTF8 validation and case folding is expensive and most use-cases +do not really need that. If there's a specific use-case, an explicit utf8 +flag can be added back. From 90fa6f7c7d5741e5104f4acc4f01f958898390b8 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 17:32:02 +0100 Subject: [PATCH 23/47] str-utils: extract character class helpers from syslog-format Signed-off-by: Balazs Scheidler --- lib/str-utils.h | 18 +++++++++++++ modules/syslogformat/syslog-format.c | 38 ++++++++-------------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/str-utils.h b/lib/str-utils.h index 2981fbfbfa..7a97315eac 100644 --- a/lib/str-utils.h +++ b/lib/str-utils.h @@ -154,4 +154,22 @@ gchar **strsplit(const gchar *str, char delim, gint maxtokens) return (gchar **) g_ptr_array_free(array, FALSE); } +static inline gboolean +ch_isdigit(gchar c) +{ + return c >= '0' && c <= '9'; +} + +static inline gboolean +ch_isalpha(gchar c) +{ + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); +} + +static inline gboolean +ch_isascii(gchar c) +{ + return c >= 0 && c <= 127; +} + #endif diff --git a/modules/syslogformat/syslog-format.c b/modules/syslogformat/syslog-format.c index 48ce92f5a9..cf7ef993c5 100644 --- a/modules/syslogformat/syslog-format.c +++ b/modules/syslogformat/syslog-format.c @@ -62,24 +62,6 @@ _skip_char(const guchar **data, gint *left) return TRUE; } -static inline gboolean -_isdigit(char c) -{ - return c >= '0' && c <= '9'; -} - -static inline gboolean -_isalpha(char c) -{ - return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); -} - -static inline gboolean -_isascii(char c) -{ - return c >= 0 && c <= 127; -} - static gint _skip_chars(const guchar **data, gint *length, const gchar *chars, gint max_len) { @@ -150,7 +132,7 @@ _syslog_format_parse_pri(LogMessage *msg, const guchar **data, gint *length, gui pri = 0; while (left && *src != '>') { - if (_isdigit(*src)) + if (ch_isdigit(*src)) { pri = pri * 10 + ((*src) - '0'); } @@ -217,7 +199,7 @@ _syslog_format_parse_cisco_sequence_id(LogMessage *msg, const guchar **data, gin while (left && *src != ':') { - if (!_isdigit(*src)) + if (!ch_isdigit(*src)) return; if (!_skip_char(&src, &left)) return; @@ -325,7 +307,7 @@ _syslog_format_parse_version(LogMessage *msg, const guchar **data, gint *length) while (left && *src != ' ') { - if (_isdigit(*src)) + if (ch_isdigit(*src)) { version = version * 10 + ((*src) - '0'); } @@ -349,12 +331,12 @@ static gsize program_name_allowed_spacial_chars_len = G_N_ELEMENTS(program_name_ static inline gboolean _validate_program_char(const guchar ch, gboolean *has_alpha) { - if (_isalpha(ch)) + if (ch_isalpha(ch)) { *has_alpha = TRUE; return TRUE; } - if (_isdigit(ch)) + if (ch_isdigit(ch)) return TRUE; if (memchr(program_name_allowed_specials, ch, program_name_allowed_spacial_chars_len)) return TRUE; @@ -625,7 +607,7 @@ _syslog_format_parse_sd(LogMessage *msg, const guchar **data, gint *length, cons open_sd++; do { - if (!left || !_isascii(*src) || *src == '=' || *src == ' ' || *src == ']' || *src == '"') + if (!left || !ch_isascii(*src) || *src == '=' || *src == ' ' || *src == ']' || *src == '"') goto error; /* read sd_id */ pos = 0; @@ -633,7 +615,7 @@ _syslog_format_parse_sd(LogMessage *msg, const guchar **data, gint *length, cons { if (pos < sizeof(sd_id_name) - 1 - options->sdata_prefix_len) { - if (_isascii(*src) && *src != '=' && *src != ' ' && *src != ']' && *src != '"') + if (ch_isascii(*src) && *src != '=' && *src != ' ' && *src != ']' && *src != '"') { sd_id_name[pos] = *src; pos++; @@ -681,7 +663,7 @@ _syslog_format_parse_sd(LogMessage *msg, const guchar **data, gint *length, cons else goto error; - if (!left || !_isascii(*src) || *src == '=' || *src == ' ' || *src == ']' || *src == '"') + if (!left || !ch_isascii(*src) || *src == '=' || *src == ' ' || *src == ']' || *src == '"') goto error; /* read sd-param */ @@ -690,7 +672,7 @@ _syslog_format_parse_sd(LogMessage *msg, const guchar **data, gint *length, cons { if (pos < sizeof(sd_param_name) - 1 - sd_id_len) { - if (_isascii(*src) && *src != '=' && *src != ' ' && *src != ']' && *src != '"') + if (ch_isascii(*src) && *src != '=' && *src != ' ' && *src != ']' && *src != '"') { sd_param_name[pos] = *src; pos++; @@ -985,7 +967,7 @@ _syslog_format_check_framing(LogMessage *msg, const guchar **data, gint *length) gint left = *length; gint i = 0; - while (left > 0 && _isdigit(*src)) + while (left > 0 && ch_isdigit(*src)) { if (!_skip_char(&src, &left)) return; From 51dd8875cf1b8fde92961d560aa19060bf197303 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 17:43:22 +0100 Subject: [PATCH 24/47] str-utils: add ch_toupper/ch_tolower() Signed-off-by: Balazs Scheidler --- lib/str-utils.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/str-utils.h b/lib/str-utils.h index 7a97315eac..f8ea17753d 100644 --- a/lib/str-utils.h +++ b/lib/str-utils.h @@ -172,4 +172,20 @@ ch_isascii(gchar c) return c >= 0 && c <= 127; } +static inline gchar +ch_toupper(gchar c) +{ + if (c >= 'a' && c <= 'z') + return 'A' + (c - 'a'); + return c; +} + +static inline gchar +ch_tolower(gchar c) +{ + if (c >= 'A' && c <= 'Z') + return 'a' + (c - 'A'); + return c; +} + #endif From 8b0884965cd1cc19e245ceac404cdd131814f93c Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 18:09:26 +0100 Subject: [PATCH 25/47] filterx/object-string: added filterx_string_new_translated() variant Signed-off-by: Balazs Scheidler --- lib/filterx/object-string.c | 20 ++++++++++++++------ lib/filterx/object-string.h | 2 ++ lib/filterx/tests/test_object_string.c | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/filterx/object-string.c b/lib/filterx/object-string.c index baf45a10fb..6a5d2cce0c 100644 --- a/lib/filterx/object-string.c +++ b/lib/filterx/object-string.c @@ -26,7 +26,6 @@ #include "scratch-buffers.h" #include "filterx-globals.h" #include "filterx/filterx-object-istype.h" -#include "utf8utils.h" #include "str-format.h" #include "str-utils.h" @@ -143,8 +142,8 @@ _string_add(FilterXObject *s, FilterXObject *object) return filterx_string_new(buffer->str, buffer->len); } -FilterXString * -_string_new(const gchar *str, gssize str_len) +static inline FilterXString * +_string_new(const gchar *str, gssize str_len, FilterXStringTranslateFunc translate) { if (str_len < 0) str_len = strlen(str); @@ -154,7 +153,10 @@ _string_new(const gchar *str, gssize str_len) filterx_object_init_instance(&self->super, &FILTERX_TYPE_NAME(string)); self->str_len = str_len; - memcpy(self->str, str, str_len); + if (translate) + translate(self->str, str, str_len); + else + memcpy(self->str, str, str_len); self->str[str_len] = 0; return self; @@ -163,13 +165,19 @@ _string_new(const gchar *str, gssize str_len) FilterXObject * filterx_string_new(const gchar *str, gssize str_len) { - return &_string_new(str, str_len)->super; + return &_string_new(str, str_len, NULL)->super; +} + +FilterXObject * +filterx_string_new_translated(const gchar *str, gssize str_len, FilterXStringTranslateFunc translate) +{ + return &_string_new(str, str_len, translate)->super; } FilterXString * filterx_string_typed_new(const gchar *str) { - return _string_new(str, -1); + return _string_new(str, -1, NULL); } static inline gsize diff --git a/lib/filterx/object-string.h b/lib/filterx/object-string.h index 736ede0cfb..d324b3ee84 100644 --- a/lib/filterx/object-string.h +++ b/lib/filterx/object-string.h @@ -26,6 +26,7 @@ #include "filterx-object.h" typedef struct _FilterXString FilterXString; +typedef void (*FilterXStringTranslateFunc)(gchar *target, const gchar *source, gsize source_len); FILTERX_DECLARE_TYPE(string); FILTERX_DECLARE_TYPE(bytes); @@ -39,6 +40,7 @@ FilterXObject *filterx_typecast_bytes(FilterXExpr *s, FilterXObject *args[], gsi FilterXObject *filterx_typecast_protobuf(FilterXExpr *s, FilterXObject *args[], gsize args_len); FilterXObject *filterx_string_new(const gchar *str, gssize str_len); +FilterXObject *filterx_string_new_translated(const gchar *str, gssize str_len, FilterXStringTranslateFunc translate); FilterXObject *filterx_bytes_new(const gchar *str, gssize str_len); FilterXObject *filterx_protobuf_new(const gchar *str, gssize str_len); diff --git a/lib/filterx/tests/test_object_string.c b/lib/filterx/tests/test_object_string.c index 4a52c06ab7..d751bd4ddd 100644 --- a/lib/filterx/tests/test_object_string.c +++ b/lib/filterx/tests/test_object_string.c @@ -45,6 +45,20 @@ Test(filterx_string, test_filterx_object_string_maps_to_the_right_json_value) filterx_object_unref(fobj); } +static void +_translate_to_incremented(gchar *target, const gchar *source, gsize len) +{ + for (gsize i = 0; i < len; i++) + target[i] = source[i] + 1; +} + +Test(filterx_string, test_filterx_string_new_translate) +{ + FilterXObject *fobj = filterx_string_new_translated("VMS", 3, _translate_to_incremented); + assert_object_json_equals(fobj, "\"WNT\""); + filterx_object_unref(fobj); +} + Test(filterx_string, test_filterx_string_typecast_null_args) { FilterXObject *obj = filterx_typecast_string(NULL, NULL, 0); From 23af8c9e879fc2d5d237206f166686208c2734e6 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 18:10:12 +0100 Subject: [PATCH 26/47] filterx/func-str-transform: avoid memory allocation in upper() and lower() functions Signed-off-by: Balazs Scheidler --- lib/filterx/func-str-transform.c | 29 +++-- .../filterx/test_filterx_funcs.py | 107 ++++++++++++++++++ 2 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 tests/light/functional_tests/filterx/test_filterx_funcs.py diff --git a/lib/filterx/func-str-transform.c b/lib/filterx/func-str-transform.c index fe452791c6..5ab62cd4d5 100644 --- a/lib/filterx/func-str-transform.c +++ b/lib/filterx/func-str-transform.c @@ -26,6 +26,7 @@ #include "filterx/object-extractor.h" #include "filterx/object-string.h" #include "filterx/filterx-eval.h" +#include "str-utils.h" static const gchar * _extract_str_arg(FilterXExpr *s, FilterXObject *args[], gsize args_len, gssize *len) @@ -50,6 +51,24 @@ _extract_str_arg(FilterXExpr *s, FilterXObject *args[], gsize args_len, gssize * return str; } +static void +_translate_to_lower(gchar *target, const gchar *source, gsize len) +{ + for (gsize i = 0; i < len; i++) + { + target[i] = ch_tolower(source[i]); + } +} + +static void +_translate_to_upper(gchar *target, const gchar *source, gsize len) +{ + for (gsize i = 0; i < len; i++) + { + target[i] = ch_toupper(source[i]); + } +} + FilterXObject * filterx_simple_function_lower(FilterXExpr *s, FilterXObject *args[], gsize args_len) { @@ -58,10 +77,7 @@ filterx_simple_function_lower(FilterXExpr *s, FilterXObject *args[], gsize args_ if (!str) return NULL; - gchar *lower = g_utf8_strdown(str, len); - FilterXObject *result = filterx_string_new(lower, -1); - g_free(lower); - + FilterXObject *result = filterx_string_new_translated(str, len, _translate_to_lower); return result; } @@ -73,9 +89,6 @@ filterx_simple_function_upper(FilterXExpr *s, FilterXObject *args[], gsize args_ if (!str) return NULL; - gchar *upper = g_utf8_strup(str, len); - FilterXObject *result = filterx_string_new(upper, -1); - g_free(upper); - + FilterXObject *result = filterx_string_new_translated(str, len, _translate_to_upper); return result; } diff --git a/tests/light/functional_tests/filterx/test_filterx_funcs.py b/tests/light/functional_tests/filterx/test_filterx_funcs.py new file mode 100644 index 0000000000..eaab903d98 --- /dev/null +++ b/tests/light/functional_tests/filterx/test_filterx_funcs.py @@ -0,0 +1,107 @@ +#!/usr/bin/env python +############################################################################# +# Copyright (c) 2025 Balazs Scheidler +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 as published +# by the Free Software Foundation, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +# +# As an additional exemption you are allowed to compile & link against the +# OpenSSL libraries as published by the OpenSSL project. See the file +# COPYING for details. +# +############################################################################# +from src.syslog_ng_config.renderer import render_statement + + +# noqa: E122 + + +def render_filterx_exprs(expr): + return f"filterx {{ {expr} }};" + + +def create_config(config, expr, msg="foobar", template="'$MSG\n'"): + file_final = config.create_file_destination(file_name="dest-final.log", template=template) + + preamble = f""" +@version: {config.get_version()} + +options {{ stats(level(1)); }}; + +source genmsg {{ + example-msg-generator( + num(1) + template("{msg}") + values( + "values.str" => string("string"), + "values.bool" => boolean(true), + "values.int" => int(5), + "values.double" => double(32.5), + "values.datetime" => datetime("1701350398.123000+01:00"), + "values.list" => list("foo,bar,baz"), + "values.null" => null(""), + "values.bytes" => bytes("binary whatever"), + "values.protobuf" => protobuf("this is not a valid protobuf!!"), + "values.json" => json('{{"emb_key1": "emb_key1 value", "emb_key2": "emb_key2 value"}}'), + "values.true_string" => string("boolean:true"), + "values.false_string" => string("boolean:false"), + ) + ); +}}; + +destination dest_final {{ + {render_statement(file_final)}; +}}; + +log {{ + source(genmsg); + {render_filterx_exprs(expr)}; + destination(dest_final); +}}; +""" + config.set_raw_config(preamble) + return (file_final,) + + +def test_upper(config, syslog_ng): + (file_final,) = create_config( + config, r""" + variable="foobar"; + $HOST="hostname"; + $MSG = json(); + $MSG.literal = upper('abc'); + $MSG.variable = upper(variable); + $MSG.host = upper($HOST); + """, + ) + syslog_ng.start(config) + + assert file_final.get_stats()["processed"] == 1 + assert file_final.read_log() == """{"literal":"ABC","variable":"FOOBAR","host":"HOSTNAME"}\n""" + + +def test_lower(config, syslog_ng): + (file_final,) = create_config( + config, r""" + variable="FoOBaR"; + $HOST="HoStNamE"; + $MSG = json(); + $MSG.literal = lower('AbC'); + $MSG.variable = lower(variable); + $MSG.host = lower($HOST); + """, + ) + syslog_ng.start(config) + + assert file_final.get_stats()["processed"] == 1 + assert file_final.read_log() == """{"literal":"abc","variable":"foobar","host":"hostname"}\n""" From 5938ee0948d10daa3ce2d734a308d678d57a488d Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 18:11:17 +0100 Subject: [PATCH 27/47] filterx/func-str: use string length if already available Signed-off-by: Balazs Scheidler --- lib/filterx/func-str.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/filterx/func-str.c b/lib/filterx/func-str.c index f4d01fde3c..9ddaab7554 100644 --- a/lib/filterx/func-str.c +++ b/lib/filterx/func-str.c @@ -74,7 +74,7 @@ _format_str_obj(FilterXObject *obj) gsize str_len; if(!filterx_object_extract_string_ref(obj, &obj_str, &str_len)) return NULL; - g_string_assign(str, obj_str); + g_string_assign_len(str, obj_str, str_len); return str; } From c3a63121d092ffd6a1cff9d4ab31160b50b47e7f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 18:11:40 +0100 Subject: [PATCH 28/47] filterx/func-str: avoid memory allocation in ignorecase=true Signed-off-by: Balazs Scheidler --- lib/filterx/func-str.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/filterx/func-str.c b/lib/filterx/func-str.c index 9ddaab7554..02db65704e 100644 --- a/lib/filterx/func-str.c +++ b/lib/filterx/func-str.c @@ -33,6 +33,7 @@ #include "filterx/object-list-interface.h" #include "filterx/filterx-object-istype.h" #include "scratch-buffers.h" +#include "str-utils.h" #define FILTERX_FUNC_STARTSWITH_USAGE "Usage: startswith(string, prefix, ignorecase=true)" \ "or startswith(string, [prefix_1, prefix_2, ..], ignorecase=true)" @@ -79,16 +80,11 @@ _format_str_obj(FilterXObject *obj) return str; } -static gboolean +static inline gboolean _do_casefold(GString *str) { - if(str->len > G_MAXSSIZE) - return FALSE; - - gchar *casefolded_str = g_utf8_casefold(str->str, (gssize) str->len); - g_string_assign(str, casefolded_str); - - g_free(casefolded_str); + for (gsize i = 0; i < str->len; i++) + str->str[i] = ch_toupper(str->str[i]); return TRUE; } From ff927ef582dc9827691ef1416456c71318ef22af Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 17:47:49 +0100 Subject: [PATCH 29/47] filterx/filterx-scope: remove "log_msg_has_changes" Previously a coupling was established between logmsg and FilterXScope, namely whenever the LogMessage was cloned, the logmsg layer called filterx_scope_set_log_msg_has_changes(), which was then subsequently used for invalidating message tied variables in the scope. This was broken for cases where the message was writable, so it changed without being cloned. In those cases the stale variables survived anyway. Another issue was that this produced excessive calls to the expensive filterx_scope_invalidate_log_msg_cache(), as it may be filterx_scope_sync() that causes the LogMessage to be cloned. In those cases we executed both filterx_scope_sync() and an entirely unnecessary filterx_scope_invalidate_log_msg_cache() both iterating on all FilterXVariable instances in the scope. This mechanism is being replaced by the generation counter mechanism, but to make the patches easier to review, this patch just removes the entire log_msg_has_changes() mechanism. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-pipe.c | 3 --- lib/filterx/filterx-scope.c | 54 +------------------------------------ lib/filterx/filterx-scope.h | 4 --- lib/logmsg/logmsg.c | 6 ----- 4 files changed, 1 insertion(+), 66 deletions(-) diff --git a/lib/filterx/filterx-pipe.c b/lib/filterx/filterx-pipe.c index 72c3fb4f5a..b0362941b4 100644 --- a/lib/filterx/filterx-pipe.c +++ b/lib/filterx/filterx-pipe.c @@ -70,9 +70,6 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o path_options = log_path_options_chain(&local_path_options, path_options); filterx_eval_init_context(&eval_context, path_options->filterx_context, msg); - if (filterx_scope_has_log_msg_changes(eval_context.scope)) - filterx_scope_invalidate_log_msg_cache(eval_context.scope); - msg_trace(">>>>>> filterx rule evaluation begin", evt_tag_str("rule", self->name), log_pipe_location_tag(s), diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index f3d2dfad0e..1470ecc401 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -27,7 +27,7 @@ struct _FilterXScope { GAtomicCounter ref_cnt; GArray *variables; - guint32 generation:20, write_protected, dirty, syncable, log_msg_has_changes; + guint32 generation:20, write_protected, dirty, syncable; }; volatile gint filterx_scope_variables_max = 16; @@ -65,24 +65,6 @@ _lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariab return FALSE; } -void -filterx_scope_set_log_msg_has_changes(FilterXScope *self) -{ - self->log_msg_has_changes = TRUE; -} - -void -filterx_scope_clear_log_msg_has_changes(FilterXScope *self) -{ - self->log_msg_has_changes = FALSE; -} - -gboolean -filterx_scope_has_log_msg_changes(FilterXScope *self) -{ - return self->log_msg_has_changes; -} - void filterx_scope_set_dirty(FilterXScope *self) { @@ -363,37 +345,3 @@ filterx_scope_unref(FilterXScope *self) if (self && (g_atomic_counter_dec_and_test(&self->ref_cnt))) _free(self); } - -void -filterx_scope_invalidate_log_msg_cache(FilterXScope *self) -{ - g_assert(filterx_scope_has_log_msg_changes(self)); - - /* this is a bit hacky and the solution would be to get rid of the GArray - * wrapper. GArray does not allow us to remove multiple elements in a - * single loop, without moving the array multiple times. So we basically - * open code this instead of multiple calls to g_array_remove_index() - */ - - gint src_index, dst_index; - for (src_index = 0, dst_index = 0; src_index < self->variables->len; src_index++) - { - FilterXVariable *v = &g_array_index(self->variables, FilterXVariable, src_index); - - if (!filterx_variable_is_floating(v) && self->syncable) - { - /* skip this variable */ - filterx_variable_clear(v); - } - else - { - if (src_index != dst_index) - g_array_index(self->variables, FilterXVariable, dst_index) = g_array_index(self->variables, FilterXVariable, src_index); - dst_index++; - } - } - /* and this is the HACK: we reset the "len" member by poking that inside the GArray data structure */ - self->variables->len = dst_index; - - filterx_scope_clear_log_msg_has_changes(self); -} diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 2e996d2020..21d9aee29b 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -43,9 +43,6 @@ typedef struct _FilterXScope FilterXScope; typedef gboolean (*FilterXScopeForeachFunc)(FilterXVariable *variable, gpointer user_data); -void filterx_scope_set_log_msg_has_changes(FilterXScope *self); -void filterx_scope_clear_log_msg_has_changes(FilterXScope *self); -gboolean filterx_scope_has_log_msg_changes(FilterXScope *self); void filterx_scope_set_dirty(FilterXScope *self); gboolean filterx_scope_is_dirty(FilterXScope *self); void filterx_scope_sync(FilterXScope *self, LogMessage *msg); @@ -56,7 +53,6 @@ FilterXVariable *filterx_scope_register_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXObject *initial_value); gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data); -void filterx_scope_invalidate_log_msg_cache(FilterXScope *self); /* copy on write */ void filterx_scope_write_protect(FilterXScope *self); diff --git a/lib/logmsg/logmsg.c b/lib/logmsg/logmsg.c index 9eab675955..139e9254d0 100644 --- a/lib/logmsg/logmsg.c +++ b/lib/logmsg/logmsg.c @@ -296,15 +296,9 @@ log_msg_make_writable(LogMessage **pself, const LogPathOptions *path_options) log_msg_unref(*pself); *pself = new; } - if(path_options->filterx_context && path_options->filterx_context->scope) - { - filterx_scope_make_writable(&path_options->filterx_context->scope); - filterx_scope_set_log_msg_has_changes(path_options->filterx_context->scope); - } return *pself; } - static void log_msg_update_sdata_slow(LogMessage *self, NVHandle handle, const gchar *name, gssize name_len) { From 56dab33436fb941dd1decead0eaee6e79fc7e8fe Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 22:49:37 +0100 Subject: [PATCH 30/47] logmsg: make log_msg_make_writable() and related functions inline Signed-off-by: Balazs Scheidler --- lib/logmsg/logmsg.c | 20 -------------------- lib/logmsg/logmsg.h | 22 ++++++++++++++++++++-- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/lib/logmsg/logmsg.c b/lib/logmsg/logmsg.c index 139e9254d0..cc775e81d6 100644 --- a/lib/logmsg/logmsg.c +++ b/lib/logmsg/logmsg.c @@ -279,26 +279,6 @@ static StatsCounterItem *count_sdata_updates; static StatsCounterItem *count_allocated_bytes; static GPrivate priv_macro_value = G_PRIVATE_INIT(__free_macro_value); -void -log_msg_write_protect(LogMessage *self) -{ - self->write_protected = TRUE; -} - -LogMessage * -log_msg_make_writable(LogMessage **pself, const LogPathOptions *path_options) -{ - if (log_msg_is_write_protected(*pself)) - { - LogMessage *new; - - new = log_msg_clone_cow(*pself, path_options); - log_msg_unref(*pself); - *pself = new; - } - return *pself; -} - static void log_msg_update_sdata_slow(LogMessage *self, NVHandle handle, const gchar *name, gssize name_len) { diff --git a/lib/logmsg/logmsg.h b/lib/logmsg/logmsg.h index 5d9d588a12..a77b69b0ee 100644 --- a/lib/logmsg/logmsg.h +++ b/lib/logmsg/logmsg.h @@ -329,7 +329,12 @@ extern gint logmsg_node_max; LogMessage *log_msg_ref(LogMessage *m); void log_msg_unref(LogMessage *m); -void log_msg_write_protect(LogMessage *m); + +static inline void +log_msg_write_protect(LogMessage *self) +{ + self->write_protected = TRUE; +} static inline gboolean log_msg_is_write_protected(const LogMessage *self) @@ -338,7 +343,20 @@ log_msg_is_write_protected(const LogMessage *self) } LogMessage *log_msg_clone_cow(LogMessage *msg, const LogPathOptions *path_options); -LogMessage *log_msg_make_writable(LogMessage **pmsg, const LogPathOptions *path_options); + +static inline LogMessage * +log_msg_make_writable(LogMessage **pself, const LogPathOptions *path_options) +{ + if (log_msg_is_write_protected(*pself)) + { + LogMessage *new_msg; + + new_msg = log_msg_clone_cow(*pself, path_options); + log_msg_unref(*pself); + *pself = new_msg; + } + return *pself; +} gboolean log_msg_write(LogMessage *self, SerializeArchive *sa); gboolean log_msg_read(LogMessage *self, SerializeArchive *sa); From ddd99986264c615c9a74a5edab0b990eb03d4cdc Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 09:07:12 +0100 Subject: [PATCH 31/47] filterx/filterx-scope: publish FilterXScope to make it possible to use inline functions Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 6 ------ lib/filterx/filterx-scope.h | 6 ++++++ lib/filterx/tests/test_filterx_expr.c | 6 ------ 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 1470ecc401..39c8c9e0cd 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -23,12 +23,6 @@ #include "filterx/filterx-scope.h" #include "scratch-buffers.h" -struct _FilterXScope -{ - GAtomicCounter ref_cnt; - GArray *variables; - guint32 generation:20, write_protected, dirty, syncable; -}; volatile gint filterx_scope_variables_max = 16; diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 21d9aee29b..b65d9b6d1f 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -40,6 +40,12 @@ */ typedef struct _FilterXScope FilterXScope; +struct _FilterXScope +{ + GAtomicCounter ref_cnt; + GArray *variables; + guint32 generation:20, write_protected, dirty, syncable; +}; typedef gboolean (*FilterXScopeForeachFunc)(FilterXVariable *variable, gpointer user_data); diff --git a/lib/filterx/tests/test_filterx_expr.c b/lib/filterx/tests/test_filterx_expr.c index 299ed047e1..84508381aa 100644 --- a/lib/filterx/tests/test_filterx_expr.c +++ b/lib/filterx/tests/test_filterx_expr.c @@ -86,12 +86,6 @@ Test(filterx_expr, test_filterx_template_evaluates_to_the_expanded_value) filterx_object_unref(fobj); } -struct _FilterXScope -{ - GHashTable *value_cache; - GPtrArray *weak_refs; -}; - Test(filterx_expr, test_filterx_list_merge) { // $fillable = json_array(); From 0973f158e334d90ca5a4ddcc4b1303b10e4a4c12 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 08:10:30 +0100 Subject: [PATCH 32/47] filterx/filterx-scope: associate FilterXScope to the message being processed I want to delegate the responsibility of tracking LogMessage changes to the scope (just as floating values are tracked by it), and as a preparation add a "msg" member to FilterXScope and make sure it always contains the right message. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-eval.c | 1 + lib/filterx/filterx-scope.c | 3 +++ lib/filterx/filterx-scope.h | 9 +++++++++ 3 files changed, 13 insertions(+) diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c index eb815b4c91..b9b93d3d14 100644 --- a/lib/filterx/filterx-eval.c +++ b/lib/filterx/filterx-eval.c @@ -154,6 +154,7 @@ filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previ else scope = filterx_scope_new(); filterx_scope_make_writable(&scope); + filterx_scope_set_message(scope, msg); memset(context, 0, sizeof(*context)); context->msg = msg; diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 39c8c9e0cd..e14a97b373 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -279,6 +279,7 @@ filterx_scope_clone(FilterXScope *other) if (other->variables->len > 0) self->dirty = other->dirty; self->syncable = other->syncable; + self->msg = log_msg_ref(other->msg); msg_trace("Filterx clone finished", evt_tag_printf("scope", "%p", self), @@ -321,6 +322,8 @@ _free(FilterXScope *self) if (variables_max < self->variables->len) filterx_scope_variables_max = self->variables->len; + if (self->msg) + log_msg_unref(self->msg); g_array_free(self->variables, TRUE); g_free(self); } diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index b65d9b6d1f..20a69a1e65 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -43,6 +43,7 @@ typedef struct _FilterXScope FilterXScope; struct _FilterXScope { GAtomicCounter ref_cnt; + LogMessage *msg; GArray *variables; guint32 generation:20, write_protected, dirty, syncable; }; @@ -68,4 +69,12 @@ FilterXScope *filterx_scope_new(void); FilterXScope *filterx_scope_ref(FilterXScope *self); void filterx_scope_unref(FilterXScope *self); +static inline void +filterx_scope_set_message(FilterXScope *self, LogMessage *msg) +{ + if (self->msg) + log_msg_unref(self->msg); + self->msg = log_msg_ref(msg); +} + #endif From ada6ab6cc3af092ffe255d6198a8fd30010cbda6 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 14:02:02 +0100 Subject: [PATCH 33/47] filterx/filterx-scope: make filterx_scope_{is,set}_dirty() inline Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 12 ------------ lib/filterx/filterx-scope.h | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index e14a97b373..a95e4e64ed 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -59,18 +59,6 @@ _lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariab return FALSE; } -void -filterx_scope_set_dirty(FilterXScope *self) -{ - self->dirty = TRUE; -} - -gboolean -filterx_scope_is_dirty(FilterXScope *self) -{ - return self->dirty; -} - static gboolean _validate_variable(FilterXScope *self, FilterXVariable *variable) { diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 20a69a1e65..4d693e40a5 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -50,8 +50,6 @@ struct _FilterXScope typedef gboolean (*FilterXScopeForeachFunc)(FilterXVariable *variable, gpointer user_data); -void filterx_scope_set_dirty(FilterXScope *self); -gboolean filterx_scope_is_dirty(FilterXScope *self); void filterx_scope_sync(FilterXScope *self, LogMessage *msg); FilterXVariable *filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle); @@ -69,6 +67,18 @@ FilterXScope *filterx_scope_new(void); FilterXScope *filterx_scope_ref(FilterXScope *self); void filterx_scope_unref(FilterXScope *self); +static inline void +filterx_scope_set_dirty(FilterXScope *self) +{ + self->dirty = TRUE; +} + +static inline gboolean +filterx_scope_is_dirty(FilterXScope *self) +{ + return self->dirty; +} + static inline void filterx_scope_set_message(FilterXScope *self, LogMessage *msg) { From c5834163dbe61088c2ae8ef32f2a5db455dc2d45 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 09:44:05 +0100 Subject: [PATCH 34/47] filterx/filterx-scope: implement LogMessage change tracking in FilterXScope Previously message-tied variables were managed in part within expr-variable and in part within FilterXScope. Now with the message being available in FilterXScope, we can delegate this in entirety to FilterXScope. This also implements the validation of message-tied values, so if the LogMessage changes independently from FilterXScope, we will notice that too and consider the values of those variables stale. Signed-off-by: Balazs Scheidler --- lib/filterx/expr-variable.c | 87 +++++++--------- lib/filterx/filterx-scope.c | 99 +++++++++++++------ lib/filterx/filterx-scope.h | 36 ++++++- lib/filterx/filterx-variable.c | 9 +- lib/filterx/filterx-variable.h | 24 +++-- lib/filterx/func-vars.c | 13 ++- .../filterx/test_filterx_scope.py | 32 ++++++ 7 files changed, 187 insertions(+), 113 deletions(-) diff --git a/lib/filterx/expr-variable.c b/lib/filterx/expr-variable.c index 5c698395aa..bc1e914444 100644 --- a/lib/filterx/expr-variable.c +++ b/lib/filterx/expr-variable.c @@ -40,31 +40,6 @@ typedef struct _FilterXVariableExpr guint32 handle_is_macro:1; } FilterXVariableExpr; -static FilterXObject * -_pull_variable_from_message(FilterXVariableExpr *self, FilterXEvalContext *context, LogMessage *msg) -{ - gssize value_len; - LogMessageValueType t; - const gchar *value = log_msg_get_value_if_set_with_type(msg, self->handle, &value_len, &t); - if (!value) - { - filterx_eval_push_error("No such name-value pair in the log message", &self->super, self->variable_name); - return NULL; - } - - if (self->handle_is_macro) - return filterx_message_value_new(value, value_len, t); - else - return filterx_message_value_new_borrowed(value, value_len, t); -} - -/* NOTE: unset on a variable that only exists in the LogMessage, without making the message writable */ -static void -_whiteout_variable(FilterXVariableExpr *self, FilterXEvalContext *context) -{ - filterx_scope_register_variable(context->scope, FX_VAR_MESSAGE_TIED, self->handle, NULL); -} - static FilterXObject * _eval_variable(FilterXExpr *s) { @@ -75,7 +50,7 @@ _eval_variable(FilterXExpr *s) variable = filterx_scope_lookup_variable(context->scope, self->handle); if (variable) { - FilterXObject *value = filterx_variable_get_value(variable); + FilterXObject *value = filterx_scope_get_variable(context->scope, variable); if (!value) { filterx_eval_push_error("Variable is unset", &self->super, self->variable_name); @@ -83,13 +58,12 @@ _eval_variable(FilterXExpr *s) return value; } - if (filterx_variable_handle_is_message_tied(self->handle)) + if (self->variable_type == FX_VAR_MESSAGE_TIED) { - FilterXObject *msg_ref = _pull_variable_from_message(self, context, context->msg); - if(!msg_ref) - return NULL; - filterx_scope_register_variable(context->scope, FX_VAR_MESSAGE_TIED, self->handle, msg_ref); - return msg_ref; + /* auto register message tied variables */ + variable = filterx_scope_register_variable(context->scope, self->variable_type, self->handle); + if (variable) + return filterx_variable_get_value(variable); } filterx_eval_push_error("No such variable", s, self->variable_name); @@ -100,19 +74,17 @@ static void _update_repr(FilterXExpr *s, FilterXObject *new_repr) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; - FilterXScope *scope = filterx_eval_get_scope(); - FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); + FilterXEvalContext *context = filterx_eval_get_context(); + FilterXScope *scope = context->scope; - g_assert(variable != NULL); - filterx_variable_set_value(variable, new_repr, FALSE); + FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); + filterx_scope_set_variable(scope, variable, new_repr, FALSE); } static gboolean _assign(FilterXExpr *s, FilterXObject *new_value) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; - FilterXScope *scope = filterx_eval_get_scope(); - FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); if (self->handle_is_macro) { @@ -120,17 +92,16 @@ _assign(FilterXExpr *s, FilterXObject *new_value) return FALSE; } - if (!variable) - { - /* NOTE: we pass NULL as initial_value to make sure the new variable - * is considered changed due to the assignment */ + FilterXEvalContext *context = filterx_eval_get_context(); + FilterXScope *scope = context->scope; + FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); - variable = filterx_scope_register_variable(scope, self->variable_type, self->handle, NULL); - } + if (!variable) + variable = filterx_scope_register_variable(scope, self->variable_type, self->handle); /* this only clones mutable objects */ new_value = filterx_object_clone(new_value); - filterx_variable_set_value(variable, new_value, TRUE); + filterx_scope_set_variable(scope, variable, new_value, TRUE); filterx_object_unref(new_value); return TRUE; } @@ -139,15 +110,17 @@ static gboolean _isset(FilterXExpr *s) { FilterXVariableExpr *self = (FilterXVariableExpr *) s; - FilterXScope *scope = filterx_eval_get_scope(); + FilterXEvalContext *context = filterx_eval_get_context(); + FilterXScope *scope = context->scope; + LogMessage *msg = context->msg; FilterXVariable *variable = filterx_scope_lookup_variable(scope, self->handle); if (variable) return filterx_variable_is_set(variable); - FilterXEvalContext *context = filterx_eval_get_context(); - LogMessage *msg = context->msg; - return log_msg_is_value_set(msg, self->handle); + if (self->variable_type == FX_VAR_MESSAGE_TIED) + return log_msg_is_value_set(msg, filterx_variable_handle_to_nv_handle(self->handle)); + return FALSE; } static gboolean @@ -162,17 +135,25 @@ _unset(FilterXExpr *s) } FilterXEvalContext *context = filterx_eval_get_context(); + FilterXScope *scope = context->scope; + LogMessage *msg = context->msg; FilterXVariable *variable = filterx_scope_lookup_variable(context->scope, self->handle); if (variable) { - filterx_variable_unset_value(variable); + filterx_scope_unset_variable(scope, variable); return TRUE; } - LogMessage *msg = context->msg; - if (log_msg_is_value_set(msg, self->handle)) - _whiteout_variable(self, context); + if (self->variable_type == FX_VAR_MESSAGE_TIED) + { + if (log_msg_is_value_set(msg, self->handle)) + { + FilterXVariable *v = filterx_scope_register_variable(context->scope, self->variable_type, self->handle); + /* make sure it is considered changed */ + filterx_scope_unset_variable(context->scope, v); + } + } return TRUE; } diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index a95e4e64ed..3cf0e4e3a0 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -21,6 +21,7 @@ * */ #include "filterx/filterx-scope.h" +#include "filterx/object-message-value.h" #include "scratch-buffers.h" @@ -62,42 +63,28 @@ _lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariab static gboolean _validate_variable(FilterXScope *self, FilterXVariable *variable) { + if (filterx_variable_is_declared(variable)) + return TRUE; + if (filterx_variable_is_floating(variable) && - !filterx_variable_is_declared(variable) && !filterx_variable_is_same_generation(variable, self->generation)) return FALSE; - return TRUE; -} - -FilterXVariable * -filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) -{ - FilterXVariable *v; - if (_lookup_variable(self, handle, &v) && _validate_variable(self, v)) - return v; - return NULL; + if (filterx_variable_is_message_tied(variable) && + !filterx_variable_is_same_generation(variable, self->msg->generation)) + return FALSE; + return TRUE; } static FilterXVariable * _register_variable(FilterXScope *self, FilterXVariableType variable_type, - FilterXVariableHandle handle, - FilterXObject *initial_value) + FilterXVariableHandle handle) { FilterXVariable *v_slot; if (_lookup_variable(self, handle, &v_slot)) { - /* already present */ - if (!filterx_variable_is_same_generation(v_slot, self->generation)) - { - /* existing value is from a previous generation, override it as if - * it was a new value */ - - filterx_variable_set_generation(v_slot, self->generation); - filterx_variable_set_value(v_slot, initial_value, FALSE); - } return v_slot; } /* turn v_slot into an index */ @@ -105,26 +92,68 @@ _register_variable(FilterXScope *self, g_assert(v_index <= self->variables->len); g_assert(&g_array_index(self->variables, FilterXVariable, v_index) == v_slot); - + /* we register an unset variable here first */ FilterXVariable v; - filterx_variable_init_instance(&v, variable_type, handle, initial_value, self->generation); + filterx_variable_init_instance(&v, variable_type, handle); g_array_insert_val(self->variables, v_index, v); return &g_array_index(self->variables, FilterXVariable, v_index); } +FilterXVariable * +filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) +{ + FilterXVariable *v; + + if (_lookup_variable(self, handle, &v) && + _validate_variable(self, v)) + return v; + return NULL; +} + +static FilterXObject * +_pull_variable_from_message(FilterXScope *self, NVHandle handle) +{ + gssize value_len; + LogMessageValueType t; + + const gchar *value = log_msg_get_value_if_set_with_type(self->msg, handle, &value_len, &t); + if (!value) + return NULL; + + if (log_msg_is_handle_macro(handle)) + { + FilterXObject *res = filterx_message_value_new(value, value_len, t); + filterx_object_make_readonly(res); + return res; + } + else + return filterx_message_value_new_borrowed(value, value_len, t); +} + + FilterXVariable * filterx_scope_register_variable(FilterXScope *self, FilterXVariableType variable_type, - FilterXVariableHandle handle, - FilterXObject *initial_value) + FilterXVariableHandle handle) { - FilterXVariable *v = _register_variable(self, variable_type, handle, initial_value); + FilterXVariable *v = _register_variable(self, variable_type, handle); /* the scope needs to be synced with the message if it holds a * message-tied variable (e.g. $MSG) */ - if (filterx_variable_handle_is_message_tied(handle)) - self->syncable = TRUE; + if (variable_type == FX_VAR_MESSAGE_TIED) + { + FilterXObject *value = _pull_variable_from_message(self, filterx_variable_handle_to_nv_handle(handle)); + self->syncable = TRUE; + + /* NOTE: value may be NULL on an error, in that case the variable becomes an unset one */ + filterx_variable_set_value(v, value, FALSE, self->msg->generation); + filterx_object_unref(value); + } + else + { + filterx_variable_set_value(v, NULL, FALSE, self->generation); + } return v; } @@ -155,7 +184,7 @@ filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, void filterx_scope_sync(FilterXScope *self, LogMessage *msg) { - + filterx_scope_set_message(self, msg); if (!self->dirty) { msg_trace("Filterx sync: not syncing as scope is not dirty", @@ -172,6 +201,8 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) GString *buffer = scratch_buffers_alloc(); + gint msg_generation = msg->generation; + for (gint i = 0; i < self->variables->len; i++) { FilterXVariable *v = &g_array_index(self->variables, FilterXVariable, i); @@ -199,16 +230,19 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) } else if (v->value == NULL) { + g_assert(v->generation == msg_generation); msg_trace("Filterx sync: whiteout variable, unsetting in message", evt_tag_str("variable", log_msg_get_value_name(filterx_variable_get_nv_handle(v), NULL))); /* we need to unset */ log_msg_unset_value(msg, filterx_variable_get_nv_handle(v)); filterx_variable_unassign(v); + v->generation++; } else if (filterx_variable_is_assigned(v) || filterx_object_is_modified_in_place(v->value)) { LogMessageValueType t; + g_assert(v->generation == msg_generation); msg_trace("Filterx sync: changed variable in scope, overwriting in message", evt_tag_str("variable", log_msg_get_value_name(filterx_variable_get_nv_handle(v), NULL))); @@ -218,13 +252,17 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) log_msg_set_value_with_type(msg, filterx_variable_get_nv_handle(v), buffer->str, buffer->len, t); filterx_object_set_modified_in_place(v->value, FALSE); filterx_variable_unassign(v); + v->generation++; } else { msg_trace("Filterx sync: variable in scope and message in sync, not doing anything", evt_tag_str("variable", log_msg_get_value_name(filterx_variable_get_nv_handle(v), NULL))); + v->generation++; } } + /* FIXME: hack ! */ + msg->generation = msg_generation + 1; self->dirty = FALSE; } @@ -298,7 +336,6 @@ filterx_scope_make_writable(FilterXScope **pself) *pself = new; } (*pself)->generation++; - g_assert((*pself)->generation < FILTERX_SCOPE_MAX_GENERATION); return *pself; } diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 4d693e40a5..882d415cd8 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -43,9 +43,10 @@ typedef struct _FilterXScope FilterXScope; struct _FilterXScope { GAtomicCounter ref_cnt; + guint16 write_protected:1, dirty:1, syncable:1; + FilterXGenCounter generation; LogMessage *msg; GArray *variables; - guint32 generation:20, write_protected, dirty, syncable; }; typedef gboolean (*FilterXScopeForeachFunc)(FilterXVariable *variable, gpointer user_data); @@ -55,8 +56,7 @@ void filterx_scope_sync(FilterXScope *self, LogMessage *msg); FilterXVariable *filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle); FilterXVariable *filterx_scope_register_variable(FilterXScope *self, FilterXVariableType variable_type, - FilterXVariableHandle handle, - FilterXObject *initial_value); + FilterXVariableHandle handle); gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data); /* copy on write */ @@ -79,6 +79,36 @@ filterx_scope_is_dirty(FilterXScope *self) return self->dirty; } +static inline FilterXObject * +filterx_scope_get_variable(FilterXScope *self, FilterXVariable *v) +{ + return filterx_variable_get_value(v); +} + +static inline void +filterx_scope_set_variable(FilterXScope *self, FilterXVariable *v, FilterXObject *value, gboolean assignment) +{ + if (filterx_variable_is_floating(v)) + { + G_STATIC_ASSERT(sizeof(v->generation) == sizeof(self->generation)); + filterx_variable_set_value(v, value, assignment, self->generation); + } + else + { + G_STATIC_ASSERT(sizeof(v->generation) == sizeof(self->msg->generation)); + filterx_variable_set_value(v, value, assignment, self->msg->generation); + } +} + +static inline void +filterx_scope_unset_variable(FilterXScope *self, FilterXVariable *v) +{ + if (filterx_variable_is_floating(v)) + filterx_variable_unset_value(v, self->generation); + else + filterx_variable_unset_value(v, self->msg->generation); +} + static inline void filterx_scope_set_message(FilterXScope *self, LogMessage *msg) { diff --git a/lib/filterx/filterx-variable.c b/lib/filterx/filterx-variable.c index 3b5b726e2d..6a2499f6f4 100644 --- a/lib/filterx/filterx-variable.c +++ b/lib/filterx/filterx-variable.c @@ -47,13 +47,10 @@ filterx_variable_clear(FilterXVariable *v) void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableType variable_type, - FilterXVariableHandle handle, - FilterXObject *initial_value, - guint32 generation) + FilterXVariableHandle handle) { - v->handle = handle; v->variable_type = variable_type; + v->handle = handle; v->assigned = FALSE; - v->generation = generation; - v->value = filterx_object_ref(initial_value); + v->value = NULL; } diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index b31afe52ce..11067cb243 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -36,9 +36,8 @@ typedef enum FX_VAR_DECLARED_FLOATING, } FilterXVariableType; -#define FILTERX_SCOPE_MAX_GENERATION ((1UL << 20) - 1) - typedef guint32 FilterXVariableHandle; +typedef guint16 FilterXGenCounter; #define FILTERX_HANDLE_FLOATING_BIT (1UL << 31) @@ -71,17 +70,15 @@ typedef struct _FilterXVariable * * declared -- this variable is declared (e.g. retained for the entire input pipeline) */ - guint32 assigned:1, - variable_type:2, - generation:20; + guint16 assigned:1, + variable_type:2; + FilterXGenCounter generation; FilterXObject *value; } FilterXVariable; void filterx_variable_init_instance(FilterXVariable *v, FilterXVariableType variable_type, - FilterXVariableHandle handle, - FilterXObject *initial_value, - guint32 generation); + FilterXVariableHandle handle); void filterx_variable_clear(FilterXVariable *v); static inline gboolean @@ -115,17 +112,18 @@ filterx_variable_get_value(FilterXVariable *v) } static inline void -filterx_variable_set_value(FilterXVariable *v, FilterXObject *new_value, gboolean assignment) +filterx_variable_set_value(FilterXVariable *v, FilterXObject *new_value, gboolean assignment, FilterXGenCounter generation) { filterx_object_unref(v->value); v->value = filterx_object_ref(new_value); v->assigned = assignment; + v->generation = generation; } static inline void -filterx_variable_unset_value(FilterXVariable *v) +filterx_variable_unset_value(FilterXVariable *v, FilterXGenCounter generation) { - filterx_variable_set_value(v, NULL, TRUE); + filterx_variable_set_value(v, NULL, TRUE, generation); } static inline gboolean @@ -147,13 +145,13 @@ filterx_variable_is_assigned(FilterXVariable *v) } static inline gboolean -filterx_variable_is_same_generation(FilterXVariable *v, guint32 generation) +filterx_variable_is_same_generation(FilterXVariable *v, FilterXGenCounter generation) { return v->generation == generation; } static inline void -filterx_variable_set_generation(FilterXVariable *v, guint32 generation) +filterx_variable_set_generation(FilterXVariable *v, FilterXGenCounter generation) { v->generation = generation; } diff --git a/lib/filterx/func-vars.c b/lib/filterx/func-vars.c index d412998db5..2c1db420b5 100644 --- a/lib/filterx/func-vars.c +++ b/lib/filterx/func-vars.c @@ -114,19 +114,18 @@ _load_from_dict(FilterXObject *key, FilterXObject *value, gpointer user_data) FilterXVariableType variable_type = (key_str[0] == '$') ? FX_VAR_MESSAGE_TIED : FX_VAR_DECLARED_FLOATING; FilterXVariableHandle handle = filterx_map_varname_to_handle(key_str, variable_type); - FilterXVariable *variable = NULL; - variable = filterx_scope_register_variable(scope, variable_type, handle, NULL); - - FilterXObject *cloned_value = filterx_object_clone(value); - filterx_variable_set_value(variable, cloned_value, TRUE); - filterx_object_unref(cloned_value); - + FilterXVariable *variable = filterx_scope_register_variable(scope, variable_type, handle); if (!variable) { filterx_eval_push_error("Failed to register variable", NULL, key); return FALSE; } + FilterXObject *cloned_value = filterx_object_clone(value); + filterx_scope_set_variable(scope, variable, cloned_value, TRUE); + filterx_object_unref(cloned_value); + + if (debug_flag) { LogMessageValueType type; diff --git a/tests/light/functional_tests/filterx/test_filterx_scope.py b/tests/light/functional_tests/filterx/test_filterx_scope.py index 78f8d4c6e0..256beef1c8 100644 --- a/tests/light/functional_tests/filterx/test_filterx_scope.py +++ b/tests/light/functional_tests/filterx/test_filterx_scope.py @@ -188,6 +188,38 @@ def test_message_tied_variables_do_not_propagate_to_parallel_branches(config, sy assert file_false.read_log() == "kecske\n" +def test_message_tied_variables_are_invalidated_if_message_is_changed(config, syslog_ng): + (file_true, file_false, file_final) = create_config( + config, init_exprs=[ + """ + declare foo = $MSG; + foo; + $MSG; + foo == "foobar"; + $MSG == "foobar"; + """, + ], init_log_exprs=[ + """ + rewrite { + set("foobar replacement" value("MSG")); + }; + """ + ], true_exprs=[ + """ + $MSG; + foo; + $MSG == "foobar replacement"; + foo == "foobar"; + """, + ], + ) + syslog_ng.start(config) + + assert file_true.get_stats()["processed"] == 1 + assert "processed" not in file_false.get_stats() + assert file_true.read_log() == "foobar replacement\n" + + def test_message_tied_variables_are_not_considered_changed_just_by_unmarshaling(config, syslog_ng): (file_true, file_false, file_final) = create_config( config, init_exprs=[ From 4c63f8a51940f80b2b55ed07ca9a1db1880a46c7 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 14:02:51 +0100 Subject: [PATCH 35/47] filterx/filterx-scope: make dirty tracking more accurate A scope is only considered dirty if it has message-tied variables that are changed. In any other case it's not dirty, so no sync is needed. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-eval.c | 1 - lib/filterx/filterx-scope.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c index b9b93d3d14..ddd1fca3e8 100644 --- a/lib/filterx/filterx-eval.c +++ b/lib/filterx/filterx-eval.c @@ -140,7 +140,6 @@ filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr) filterx_object_unref(res); /* NOTE: we only store the results into the message if the entire evaluation was successful */ fail: - filterx_scope_set_dirty(context->scope); return result; } diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 882d415cd8..63b75b6377 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -96,6 +96,8 @@ filterx_scope_set_variable(FilterXScope *self, FilterXVariable *v, FilterXObject else { G_STATIC_ASSERT(sizeof(v->generation) == sizeof(self->msg->generation)); + if (assignment) + filterx_scope_set_dirty(self); filterx_variable_set_value(v, value, assignment, self->msg->generation); } } From 72b505946a1500192758686617887f29b30f24a5 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 17:29:52 +0100 Subject: [PATCH 36/47] filterx/filterx-scope: add parent_scope tracking Link scopes together. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-eval.c | 2 +- lib/filterx/filterx-scope.c | 6 ++++-- lib/filterx/filterx-scope.h | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c index ddd1fca3e8..2d49f44443 100644 --- a/lib/filterx/filterx-eval.c +++ b/lib/filterx/filterx-eval.c @@ -151,7 +151,7 @@ filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previ if (previous_context) scope = filterx_scope_ref(previous_context->scope); else - scope = filterx_scope_new(); + scope = filterx_scope_new(NULL); filterx_scope_make_writable(&scope); filterx_scope_set_message(scope, msg); diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 3cf0e4e3a0..4f78c9f6f8 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -267,20 +267,21 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) } FilterXScope * -filterx_scope_new(void) +filterx_scope_new(FilterXScope *parent_scope) { FilterXScope *self = g_new0(FilterXScope, 1); g_atomic_counter_set(&self->ref_cnt, 1); self->variables = g_array_sized_new(FALSE, TRUE, sizeof(FilterXVariable), filterx_scope_variables_max); g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_clear); + self->parent_scope = filterx_scope_ref(parent_scope); return self; } static FilterXScope * filterx_scope_clone(FilterXScope *other) { - FilterXScope *self = filterx_scope_new(); + FilterXScope *self = filterx_scope_new(other); for (gint src_index = 0, dst_index = 0; src_index < other->variables->len; src_index++) { @@ -350,6 +351,7 @@ _free(FilterXScope *self) if (self->msg) log_msg_unref(self->msg); g_array_free(self->variables, TRUE); + filterx_scope_unref(self->parent_scope); g_free(self); } diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 63b75b6377..97723a384e 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -46,6 +46,7 @@ struct _FilterXScope guint16 write_protected:1, dirty:1, syncable:1; FilterXGenCounter generation; LogMessage *msg; + FilterXScope *parent_scope; GArray *variables; }; @@ -63,7 +64,7 @@ gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachF void filterx_scope_write_protect(FilterXScope *self); FilterXScope *filterx_scope_make_writable(FilterXScope **pself); -FilterXScope *filterx_scope_new(void); +FilterXScope *filterx_scope_new(FilterXScope *parent_scope); FilterXScope *filterx_scope_ref(FilterXScope *self); void filterx_scope_unref(FilterXScope *self); From 7aa427383499898cd542abbdd322f42546e6ff11 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 22:49:23 +0100 Subject: [PATCH 37/47] filterx/filterx-scope: retain the generation counter across clone operations Previously generations was reset to 0 in case we ended up doing a clone. Let's retain that instead, so we don't need to adjust the generation value for FilterXValues either. This is a preparation for sharing FilterXVariable descriptors so that we don't have to clone them. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 4f78c9f6f8..97436b2238 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -292,7 +292,6 @@ filterx_scope_clone(FilterXScope *other) g_array_append_val(self->variables, *v); FilterXVariable *v_clone = &g_array_index(self->variables, FilterXVariable, dst_index); - filterx_variable_set_generation(v_clone, 0); if (v->value) v_clone->value = filterx_object_clone(v->value); else @@ -302,7 +301,8 @@ filterx_scope_clone(FilterXScope *other) evt_tag_str("variable", log_msg_get_value_name((filterx_variable_get_nv_handle(v)), NULL))); } } - + /* retain the generation counter */ + self->generation = other->generation; if (other->variables->len > 0) self->dirty = other->dirty; self->syncable = other->syncable; From 82b42037893c5db526c64fff0f160cf8239d6840 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 22:32:57 +0100 Subject: [PATCH 38/47] filterx/filterx-scope: implement delayed variable cloning Instead of cloning all variables into subsequent scopes, let's start with an empty array and only clone the ones that are actually used. This improves performance a lot in our use-cases. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 47 +++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 97436b2238..0ba60962a9 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -100,6 +100,20 @@ _register_variable(FilterXScope *self, return &g_array_index(self->variables, FilterXVariable, v_index); } +static FilterXVariable * +_pull_variable_from_parent_scope(FilterXScope *self, FilterXScope *scope, FilterXVariable *previous_v) +{ + FilterXVariable *v = _register_variable(self, previous_v->variable_type, previous_v->handle); + + *v = *previous_v; + if (v->value) + v->value = filterx_object_clone(v->value); + + msg_trace("Filterx scope, cloning scope variable", + evt_tag_str("variable", log_msg_get_value_name((filterx_variable_get_nv_handle(v)), NULL))); + return v; +} + FilterXVariable * filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) { @@ -108,6 +122,21 @@ filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) if (_lookup_variable(self, handle, &v) && _validate_variable(self, v)) return v; + + for (FilterXScope *scope = self->parent_scope; scope; scope = scope->parent_scope) + { + if (_lookup_variable(scope, handle, &v)) + { + /* NOTE: we validate against @self */ + if (_validate_variable(self, v)) + return _pull_variable_from_parent_scope(self, scope, v); + else + msg_trace("Filterx scope, not cloning stale scope variable", + evt_tag_str("variable", log_msg_get_value_name((filterx_variable_get_nv_handle(v)), NULL))); + + return NULL; + } + } return NULL; } @@ -283,24 +312,6 @@ filterx_scope_clone(FilterXScope *other) { FilterXScope *self = filterx_scope_new(other); - for (gint src_index = 0, dst_index = 0; src_index < other->variables->len; src_index++) - { - FilterXVariable *v = &g_array_index(other->variables, FilterXVariable, src_index); - - if (filterx_variable_is_declared(v) || filterx_variable_is_message_tied(v)) - { - g_array_append_val(self->variables, *v); - FilterXVariable *v_clone = &g_array_index(self->variables, FilterXVariable, dst_index); - - if (v->value) - v_clone->value = filterx_object_clone(v->value); - else - v_clone->value = NULL; - dst_index++; - msg_trace("Filterx scope, cloning scope variable", - evt_tag_str("variable", log_msg_get_value_name((filterx_variable_get_nv_handle(v)), NULL))); - } - } /* retain the generation counter */ self->generation = other->generation; if (other->variables->len > 0) From b9b5709f47f6ea6a0c170d498ac3624820334b4a Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 15:21:23 +0100 Subject: [PATCH 39/47] filterx/filterx-scope: allocate variables inline Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-scope.c | 135 +++++++++++++++++++++++++-------- lib/filterx/filterx-scope.h | 15 +++- lib/filterx/filterx-variable.c | 2 - 3 files changed, 115 insertions(+), 37 deletions(-) diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 0ba60962a9..4bb9b93816 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -27,24 +27,85 @@ volatile gint filterx_scope_variables_max = 16; +static inline FilterXVariable * +_get_variable_array(FilterXScope *self) +{ + if (self->coupled_variables) + return self->variables.coupled; + else + return self->variables.separate; +} + +static FilterXVariable * +_insert_into_large_enough_array(FilterXScope *self, FilterXVariable *elems, gint index) +{ + g_assert(self->variables.size >= self->variables.len + 1); + + gint n = self->variables.len - index; + if (n) + memmove(&elems[index + 1], &elems[index], n * sizeof(elems[0])); + self->variables.len++; + elems[index] = (FilterXVariable){0}; + return &elems[index]; +} + +static void +_separate_variables(FilterXScope *self) +{ + /* realloc */ + FilterXVariable *separate_array = g_new(FilterXVariable, self->variables.size * 2); + + memcpy(separate_array, self->variables.coupled, self->variables.len * sizeof(separate_array[0])); + self->coupled_variables = FALSE; + self->variables.size *= 2; + self->variables.separate = separate_array; +} + +static void +_grow_separate_array(FilterXScope *self) +{ + self->variables.size *= 2; + self->variables.separate = g_realloc(self->variables.separate, self->variables.size * sizeof(self->variables.separate[0])); +} + +static FilterXVariable * +_insert_variable(FilterXScope *self, gint index) +{ + if (self->coupled_variables) + { + if (self->variables.len + 1 < self->variables.size) + return _insert_into_large_enough_array(self, self->variables.coupled, index); + + _separate_variables(self); + } + + if (self->variables.len + 1 >= self->variables.size) + _grow_separate_array(self); + + return _insert_into_large_enough_array(self, self->variables.separate, index); +} + static gboolean -_lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariable **v_slot) +_lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariable **v_slot, gint *v_index) { gint l, h, m; + FilterXVariable *variables = _get_variable_array(self); + gint variables_len = self->variables.len; /* open-coded binary search */ l = 0; - h = self->variables->len - 1; + h = variables_len - 1; while (l <= h) { m = (l + h) >> 1; - FilterXVariable *m_elem = &g_array_index(self->variables, FilterXVariable, m); - + FilterXVariable *m_elem = &variables[m]; FilterXVariableHandle mv = m_elem->handle; + if (mv == handle) { *v_slot = m_elem; + *v_index = m; return TRUE; } else if (mv > handle) @@ -56,7 +117,8 @@ _lookup_variable(FilterXScope *self, FilterXVariableHandle handle, FilterXVariab l = m + 1; } } - *v_slot = &g_array_index(self->variables, FilterXVariable, l); + *v_slot = &variables[l]; + *v_index = l; return FALSE; } @@ -76,28 +138,21 @@ _validate_variable(FilterXScope *self, FilterXVariable *variable) return TRUE; } + static FilterXVariable * _register_variable(FilterXScope *self, FilterXVariableType variable_type, FilterXVariableHandle handle) { - FilterXVariable *v_slot; - - if (_lookup_variable(self, handle, &v_slot)) - { - return v_slot; - } - /* turn v_slot into an index */ - gsize v_index = ((guint8 *) v_slot - (guint8 *) self->variables->data) / sizeof(FilterXVariable); - g_assert(v_index <= self->variables->len); - g_assert(&g_array_index(self->variables, FilterXVariable, v_index) == v_slot); + FilterXVariable *v; + gint v_index; - /* we register an unset variable here first */ - FilterXVariable v; - filterx_variable_init_instance(&v, variable_type, handle); - g_array_insert_val(self->variables, v_index, v); + if (_lookup_variable(self, handle, &v, &v_index)) + return v; - return &g_array_index(self->variables, FilterXVariable, v_index); + v = _insert_variable(self, v_index); + filterx_variable_init_instance(v, variable_type, handle); + return v; } static FilterXVariable * @@ -118,14 +173,15 @@ FilterXVariable * filterx_scope_lookup_variable(FilterXScope *self, FilterXVariableHandle handle) { FilterXVariable *v; + gint v_index; - if (_lookup_variable(self, handle, &v) && + if (_lookup_variable(self, handle, &v, &v_index) && _validate_variable(self, v)) return v; for (FilterXScope *scope = self->parent_scope; scope; scope = scope->parent_scope) { - if (_lookup_variable(scope, handle, &v)) + if (_lookup_variable(scope, handle, &v, &v_index)) { /* NOTE: we validate against @self */ if (_validate_variable(self, v)) @@ -189,9 +245,11 @@ filterx_scope_register_variable(FilterXScope *self, gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data) { - for (gsize i = 0; i < self->variables->len; i++) + FilterXVariable *variables = _get_variable_array(self); + + for (gint i = 0; i < self->variables.len; i++) { - FilterXVariable *variable = &g_array_index(self->variables, FilterXVariable, i); + FilterXVariable *variable = &variables[i]; if (!variable->value) continue; @@ -231,10 +289,11 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) GString *buffer = scratch_buffers_alloc(); gint msg_generation = msg->generation; + FilterXVariable *variables = _get_variable_array(self); - for (gint i = 0; i < self->variables->len; i++) + for (gint i = 0; i < self->variables.len; i++) { - FilterXVariable *v = &g_array_index(self->variables, FilterXVariable, i); + FilterXVariable *v = &variables[i]; /* we don't need to sync the value if: * @@ -298,12 +357,14 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) FilterXScope * filterx_scope_new(FilterXScope *parent_scope) { - FilterXScope *self = g_new0(FilterXScope, 1); + gsize alloc_size = sizeof(FilterXScope) + sizeof(FilterXVariable) * filterx_scope_variables_max; + FilterXScope *self = g_malloc(alloc_size); + memset(self, 0, sizeof(FilterXScope)); g_atomic_counter_set(&self->ref_cnt, 1); - self->variables = g_array_sized_new(FALSE, TRUE, sizeof(FilterXVariable), filterx_scope_variables_max); - g_array_set_clear_func(self->variables, (GDestroyNotify) filterx_variable_clear); self->parent_scope = filterx_scope_ref(parent_scope); + self->variables.size = filterx_scope_variables_max; + self->coupled_variables = TRUE; return self; } @@ -314,7 +375,7 @@ filterx_scope_clone(FilterXScope *other) /* retain the generation counter */ self->generation = other->generation; - if (other->variables->len > 0) + if (other->variables.len > 0) self->dirty = other->dirty; self->syncable = other->syncable; self->msg = log_msg_ref(other->msg); @@ -356,12 +417,20 @@ _free(FilterXScope *self) { /* NOTE: update the number of inlined variable allocations */ gint variables_max = filterx_scope_variables_max; - if (variables_max < self->variables->len) - filterx_scope_variables_max = self->variables->len; + if (variables_max < self->variables.len) + filterx_scope_variables_max = self->variables.len; if (self->msg) log_msg_unref(self->msg); - g_array_free(self->variables, TRUE); + + FilterXVariable *variables = _get_variable_array(self); + for (gint i = 0; i < self->variables.len; i++) + { + FilterXVariable *v = &variables[i]; + filterx_variable_clear(v); + } + if (!self->coupled_variables) + g_free(self->variables.separate); filterx_scope_unref(self->parent_scope); g_free(self); } diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index 97723a384e..ce7bc64899 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -43,11 +43,22 @@ typedef struct _FilterXScope FilterXScope; struct _FilterXScope { GAtomicCounter ref_cnt; - guint16 write_protected:1, dirty:1, syncable:1; + guint16 write_protected:1, dirty:1, syncable:1, coupled_variables:1; FilterXGenCounter generation; LogMessage *msg; FilterXScope *parent_scope; - GArray *variables; + struct + { + /* number of elems */ + gint len; + /* allocated elems */ + gint size; + union + { + FilterXVariable *separate; + FilterXVariable coupled[0]; + }; + } variables; }; typedef gboolean (*FilterXScopeForeachFunc)(FilterXVariable *variable, gpointer user_data); diff --git a/lib/filterx/filterx-variable.c b/lib/filterx/filterx-variable.c index 6a2499f6f4..4884064a4e 100644 --- a/lib/filterx/filterx-variable.c +++ b/lib/filterx/filterx-variable.c @@ -51,6 +51,4 @@ filterx_variable_init_instance(FilterXVariable *v, { v->variable_type = variable_type; v->handle = handle; - v->assigned = FALSE; - v->value = NULL; } From 5793da24b06319a48c5fddbf9552df3fdbb9ed04 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 23:09:40 +0100 Subject: [PATCH 40/47] filterx/filterx-scope: make it possible to allocate FilterXScope on the stack At the same time get rid of reference counting and clone. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-eval.c | 10 +---- lib/filterx/filterx-eval.h | 2 +- lib/filterx/filterx-pipe.c | 17 +++++++- lib/filterx/filterx-scope.c | 84 ++++++++++++------------------------- lib/filterx/filterx-scope.h | 34 +++++++++++---- libtest/filterx-lib.c | 5 ++- 6 files changed, 74 insertions(+), 78 deletions(-) diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c index 2d49f44443..3f069649a9 100644 --- a/lib/filterx/filterx-eval.c +++ b/lib/filterx/filterx-eval.c @@ -144,15 +144,8 @@ filterx_eval_exec(FilterXEvalContext *context, FilterXExpr *expr) } void -filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, LogMessage *msg) +filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, FilterXScope *scope, LogMessage *msg) { - FilterXScope *scope; - - if (previous_context) - scope = filterx_scope_ref(previous_context->scope); - else - scope = filterx_scope_new(NULL); - filterx_scope_make_writable(&scope); filterx_scope_set_message(scope, msg); memset(context, 0, sizeof(*context)); @@ -175,7 +168,6 @@ filterx_eval_deinit_context(FilterXEvalContext *context) { if (!context->previous_context) g_ptr_array_free(context->weak_refs, TRUE); - filterx_scope_unref(context->scope); filterx_eval_set_context(context->previous_context); } diff --git a/lib/filterx/filterx-eval.h b/lib/filterx/filterx-eval.h index 34d3a3f8a3..9f3f4f8ab9 100644 --- a/lib/filterx/filterx-eval.h +++ b/lib/filterx/filterx-eval.h @@ -68,7 +68,7 @@ EVTTAG *filterx_format_last_error_location(void); void filterx_eval_clear_errors(void); EVTTAG *filterx_format_eval_result(FilterXEvalResult result); -void filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, LogMessage *msg); +void filterx_eval_init_context(FilterXEvalContext *context, FilterXEvalContext *previous_context, FilterXScope *scope_storage, LogMessage *msg); void filterx_eval_deinit_context(FilterXEvalContext *context); static inline void diff --git a/lib/filterx/filterx-pipe.c b/lib/filterx/filterx-pipe.c index b0362941b4..72e205982c 100644 --- a/lib/filterx/filterx-pipe.c +++ b/lib/filterx/filterx-pipe.c @@ -66,9 +66,22 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o FilterXEvalContext eval_context; LogPathOptions local_path_options; FilterXEvalResult eval_res; + gboolean local_scope = FALSE; path_options = log_path_options_chain(&local_path_options, path_options); - filterx_eval_init_context(&eval_context, path_options->filterx_context, msg); + + FilterXScope *scope = NULL; + if (path_options->filterx_context) + scope = filterx_scope_reuse(path_options->filterx_context->scope); + + if (!scope) + { + gsize alloc_size = filterx_scope_get_alloc_size(); + scope = g_alloca(alloc_size); + filterx_scope_init_instance(scope, alloc_size, path_options->filterx_context ? path_options->filterx_context->scope : NULL); + local_scope = TRUE; + } + filterx_eval_init_context(&eval_context, path_options->filterx_context, scope, msg); msg_trace(">>>>>> filterx rule evaluation begin", evt_tag_str("rule", self->name), @@ -106,6 +119,8 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o } filterx_eval_deinit_context(&eval_context); + if (local_scope) + filterx_scope_clear(scope); nv_table_unref(payload); } diff --git a/lib/filterx/filterx-scope.c b/lib/filterx/filterx-scope.c index 4bb9b93816..4974781980 100644 --- a/lib/filterx/filterx-scope.c +++ b/lib/filterx/filterx-scope.c @@ -354,66 +354,34 @@ filterx_scope_sync(FilterXScope *self, LogMessage *msg) self->dirty = FALSE; } -FilterXScope * -filterx_scope_new(FilterXScope *parent_scope) -{ - gsize alloc_size = sizeof(FilterXScope) + sizeof(FilterXVariable) * filterx_scope_variables_max; - FilterXScope *self = g_malloc(alloc_size); - - memset(self, 0, sizeof(FilterXScope)); - g_atomic_counter_set(&self->ref_cnt, 1); - self->parent_scope = filterx_scope_ref(parent_scope); - self->variables.size = filterx_scope_variables_max; - self->coupled_variables = TRUE; - return self; -} - -static FilterXScope * -filterx_scope_clone(FilterXScope *other) +gsize +filterx_scope_get_alloc_size(void) { - FilterXScope *self = filterx_scope_new(other); - - /* retain the generation counter */ - self->generation = other->generation; - if (other->variables.len > 0) - self->dirty = other->dirty; - self->syncable = other->syncable; - self->msg = log_msg_ref(other->msg); - - msg_trace("Filterx clone finished", - evt_tag_printf("scope", "%p", self), - evt_tag_printf("other", "%p", other), - evt_tag_int("dirty", self->dirty), - evt_tag_int("syncable", self->syncable), - evt_tag_int("write_protected", self->write_protected)); - /* NOTE: we don't clone weak references, those only relate to mutable - * objects, which we are cloning anyway */ - return self; + return sizeof(FilterXScope) + sizeof(FilterXVariable) * filterx_scope_variables_max; } void -filterx_scope_write_protect(FilterXScope *self) +filterx_scope_init_instance(FilterXScope *storage, gsize storage_size, FilterXScope *parent_scope) { - self->write_protected = TRUE; -} + FilterXScope *self = storage; + gsize coupled_variables_size = (storage_size - sizeof(FilterXScope)) / sizeof(FilterXVariable); -FilterXScope * -filterx_scope_make_writable(FilterXScope **pself) -{ - if ((*pself)->write_protected) + memset(self, 0, sizeof(FilterXScope)); + self->variables.size = coupled_variables_size; + self->coupled_variables = TRUE; + if (parent_scope) { - FilterXScope *new; - - new = filterx_scope_clone(*pself); - filterx_scope_unref(*pself); - *pself = new; + self->parent_scope = parent_scope; + self->generation = parent_scope->generation + 1; + if (parent_scope->variables.len > 0) + self->dirty = parent_scope->dirty; + self->syncable = parent_scope->syncable; + self->msg = log_msg_ref(parent_scope->msg); } - (*pself)->generation++; - return *pself; } -static void -_free(FilterXScope *self) +void +filterx_scope_clear(FilterXScope *self) { /* NOTE: update the number of inlined variable allocations */ gint variables_max = filterx_scope_variables_max; @@ -431,21 +399,21 @@ _free(FilterXScope *self) } if (!self->coupled_variables) g_free(self->variables.separate); - filterx_scope_unref(self->parent_scope); - g_free(self); } FilterXScope * -filterx_scope_ref(FilterXScope *self) +filterx_scope_new(FilterXScope *parent_scope) { - if (self) - g_atomic_counter_inc(&self->ref_cnt); + gsize alloc_size = filterx_scope_get_alloc_size(); + FilterXScope *self = g_malloc(alloc_size); + + filterx_scope_init_instance(self, alloc_size, parent_scope); return self; } void -filterx_scope_unref(FilterXScope *self) +filterx_scope_free(FilterXScope *self) { - if (self && (g_atomic_counter_dec_and_test(&self->ref_cnt))) - _free(self); + filterx_scope_clear(self); + g_free(self); } diff --git a/lib/filterx/filterx-scope.h b/lib/filterx/filterx-scope.h index ce7bc64899..1f5cfec430 100644 --- a/lib/filterx/filterx-scope.h +++ b/lib/filterx/filterx-scope.h @@ -42,7 +42,6 @@ typedef struct _FilterXScope FilterXScope; struct _FilterXScope { - GAtomicCounter ref_cnt; guint16 write_protected:1, dirty:1, syncable:1, coupled_variables:1; FilterXGenCounter generation; LogMessage *msg; @@ -71,13 +70,11 @@ FilterXVariable *filterx_scope_register_variable(FilterXScope *self, FilterXVariableHandle handle); gboolean filterx_scope_foreach_variable(FilterXScope *self, FilterXScopeForeachFunc func, gpointer user_data); -/* copy on write */ -void filterx_scope_write_protect(FilterXScope *self); -FilterXScope *filterx_scope_make_writable(FilterXScope **pself); - +gsize filterx_scope_get_alloc_size(void); +void filterx_scope_init_instance(FilterXScope *storage, gsize storage_size, FilterXScope *parent_scope); +void filterx_scope_clear(FilterXScope *self); FilterXScope *filterx_scope_new(FilterXScope *parent_scope); -FilterXScope *filterx_scope_ref(FilterXScope *self); -void filterx_scope_unref(FilterXScope *self); +void filterx_scope_free(FilterXScope *self); static inline void filterx_scope_set_dirty(FilterXScope *self) @@ -131,4 +128,27 @@ filterx_scope_set_message(FilterXScope *self, LogMessage *msg) self->msg = log_msg_ref(msg); } +/* copy on write */ +static inline void +filterx_scope_write_protect(FilterXScope *self) +{ + self->write_protected = TRUE; +} + +static inline gboolean +filterx_scope_is_write_protected(FilterXScope *self) +{ + return self->write_protected; +} + +static inline FilterXScope * +filterx_scope_reuse(FilterXScope *self) +{ + if (filterx_scope_is_write_protected(self)) + return NULL; + + self->generation++; + return self; +} + #endif diff --git a/libtest/filterx-lib.c b/libtest/filterx-lib.c index af7441a8b9..e5d4d1c67b 100644 --- a/libtest/filterx-lib.c +++ b/libtest/filterx-lib.c @@ -193,8 +193,8 @@ init_libtest_filterx(void) FILTERX_TYPE_NAME(test_list) = FILTERX_TYPE_NAME(json_array); filterx_env.msg = create_sample_message(); - filterx_eval_init_context(&filterx_env.context, NULL, filterx_env.msg); - + filterx_env.scope = filterx_scope_new(NULL); + filterx_eval_init_context(&filterx_env.context, NULL, filterx_env.scope, filterx_env.msg); } void @@ -202,6 +202,7 @@ deinit_libtest_filterx(void) { log_msg_unref(filterx_env.msg); filterx_eval_deinit_context(&filterx_env.context); + filterx_scope_free(filterx_env.scope); } static FilterXObject * From 11efddcc1ff618383c59756a01b26bc0a199bce0 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sun, 5 Jan 2025 16:20:53 +0100 Subject: [PATCH 41/47] filterx-eval: extract context begin/end code as macros Although this code has a single user, it is relatively complex, relies on arcane mechanics of FilterXScope and FilterXEvalContext. Hide it and delegate it to show where it belongs. These have become macros, as we need to use the caller's stack frame to allocate the scope. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-eval.h | 25 ++++++++++ lib/filterx/filterx-pipe.c | 94 +++++++++++++++++--------------------- 2 files changed, 66 insertions(+), 53 deletions(-) diff --git a/lib/filterx/filterx-eval.h b/lib/filterx/filterx-eval.h index 9f3f4f8ab9..1785234285 100644 --- a/lib/filterx/filterx-eval.h +++ b/lib/filterx/filterx-eval.h @@ -121,5 +121,30 @@ filterx_eval_store_weak_ref(FilterXObject *object) } } +#define FILTERX_EVAL_BEGIN_CONTEXT(eval_context, previous_context) \ + do { \ + FilterXScope *scope = NULL; \ + gboolean local_scope = FALSE; \ + \ + if (previous_context) \ + scope = filterx_scope_reuse(previous_context->scope); \ + \ + if (!scope) \ + { \ + gsize alloc_size = filterx_scope_get_alloc_size(); \ + scope = g_alloca(alloc_size); \ + filterx_scope_init_instance(scope, alloc_size, path_options->filterx_context ? path_options->filterx_context->scope : NULL); \ + local_scope = TRUE; \ + } \ + filterx_eval_init_context(&eval_context, path_options->filterx_context, scope, msg); \ + do + + +#define FILTERX_EVAL_END_CONTEXT(eval_context) \ + while(0); \ + filterx_eval_deinit_context(&eval_context); \ + if (local_scope) \ + filterx_scope_clear(scope); \ + } while(0) #endif diff --git a/lib/filterx/filterx-pipe.c b/lib/filterx/filterx-pipe.c index 72e205982c..a891c6a4c1 100644 --- a/lib/filterx/filterx-pipe.c +++ b/lib/filterx/filterx-pipe.c @@ -64,63 +64,51 @@ log_filterx_pipe_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_o { LogFilterXPipe *self = (LogFilterXPipe *) s; FilterXEvalContext eval_context; - LogPathOptions local_path_options; - FilterXEvalResult eval_res; - gboolean local_scope = FALSE; - - path_options = log_path_options_chain(&local_path_options, path_options); - - FilterXScope *scope = NULL; - if (path_options->filterx_context) - scope = filterx_scope_reuse(path_options->filterx_context->scope); - - if (!scope) - { - gsize alloc_size = filterx_scope_get_alloc_size(); - scope = g_alloca(alloc_size); - filterx_scope_init_instance(scope, alloc_size, path_options->filterx_context ? path_options->filterx_context->scope : NULL); - local_scope = TRUE; - } - filterx_eval_init_context(&eval_context, path_options->filterx_context, scope, msg); - - msg_trace(">>>>>> filterx rule evaluation begin", - evt_tag_str("rule", self->name), - log_pipe_location_tag(s), - evt_tag_msg_reference(msg)); + FilterXEvalContext *previous_context = path_options->filterx_context; NVTable *payload = nv_table_ref(msg->payload); - eval_res = filterx_eval_exec(&eval_context, self->block); - - msg_trace("<<<<<< filterx rule evaluation result", - filterx_format_eval_result(eval_res), - evt_tag_str("rule", self->name), - log_pipe_location_tag(s), - evt_tag_int("dirty", filterx_scope_is_dirty(eval_context.scope)), - evt_tag_msg_reference(msg)); - - local_path_options.filterx_context = &eval_context; - switch (eval_res) + FILTERX_EVAL_BEGIN_CONTEXT(eval_context, previous_context) { - case FXE_SUCCESS: - log_pipe_forward_msg(s, msg, path_options); - break; - - case FXE_FAILURE: - if (path_options->matched) - (*path_options->matched) = FALSE; - /* FALLTHROUGH */ - case FXE_DROP: - log_msg_drop(msg, path_options, AT_PROCESSED); - break; - - default: - g_assert_not_reached(); - break; + FilterXEvalResult eval_res; + LogPathOptions local_path_options; + + path_options = log_path_options_chain(&local_path_options, path_options); + + msg_trace(">>>>>> filterx rule evaluation begin", + evt_tag_str("rule", self->name), + log_pipe_location_tag(s), + evt_tag_msg_reference(msg)); + + eval_res = filterx_eval_exec(&eval_context, self->block); + + msg_trace("<<<<<< filterx rule evaluation result", + filterx_format_eval_result(eval_res), + evt_tag_str("rule", self->name), + log_pipe_location_tag(s), + evt_tag_int("dirty", filterx_scope_is_dirty(eval_context.scope)), + evt_tag_msg_reference(msg)); + + local_path_options.filterx_context = &eval_context; + switch (eval_res) + { + case FXE_SUCCESS: + log_pipe_forward_msg(s, msg, path_options); + break; + + case FXE_FAILURE: + if (path_options->matched) + (*path_options->matched) = FALSE; + /* FALLTHROUGH */ + case FXE_DROP: + log_msg_drop(msg, path_options, AT_PROCESSED); + break; + + default: + g_assert_not_reached(); + break; + } } - - filterx_eval_deinit_context(&eval_context); - if (local_scope) - filterx_scope_clear(scope); + FILTERX_EVAL_END_CONTEXT(eval_context); nv_table_unref(payload); } From 0c1f824661753b262b080b81599d11734f1203be Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Fri, 3 Jan 2025 22:33:03 +0100 Subject: [PATCH 42/47] merge: FIXME --- lib/filterx/filterx-variable.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/filterx/filterx-variable.h b/lib/filterx/filterx-variable.h index 11067cb243..f893862903 100644 --- a/lib/filterx/filterx-variable.h +++ b/lib/filterx/filterx-variable.h @@ -41,6 +41,7 @@ typedef guint16 FilterXGenCounter; #define FILTERX_HANDLE_FLOATING_BIT (1UL << 31) +/* FIXME: this matches both FX_VAR_FLOATING & FX_VAR_DECLARED_FLOATING which is misleading!!! */ static inline gboolean filterx_variable_handle_is_floating(FilterXVariableHandle handle) { From 2950dc464aad234ff899086524feb206c876b9b7 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Sat, 28 Dec 2024 20:43:57 +0100 Subject: [PATCH 43/47] WIP: temporarily enable the extraction of source text --- lib/cfg-grammar.y | 4 ++-- lib/cfg-tree.c | 10 ++++++++++ lib/cfg-tree.h | 1 + lib/filterx/filterx-expr.c | 23 ++++++++++++++++++++--- lib/logmpx.c | 5 +++++ lib/logpipe.c | 17 ++++++++++++++++- 6 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lib/cfg-grammar.y b/lib/cfg-grammar.y index 59decde075..87891bce3e 100644 --- a/lib/cfg-grammar.y +++ b/lib/cfg-grammar.y @@ -567,14 +567,14 @@ expr_stmt source_stmt : KW_SOURCE string '{' source_content '}' { - $$ = log_expr_node_new_source($2, $4, &@1); + $$ = log_expr_node_new_source($2, $4, &@$); free($2); } ; dest_stmt : KW_DESTINATION string '{' dest_content '}' { - $$ = log_expr_node_new_destination($2, $4, &@1); + $$ = log_expr_node_new_destination($2, $4, &@$); free($2); } ; diff --git a/lib/cfg-tree.c b/lib/cfg-tree.c index d86b2ac635..748993c044 100644 --- a/lib/cfg-tree.c +++ b/lib/cfg-tree.c @@ -26,6 +26,7 @@ #include "logmpx.h" #include "logpipe.h" #include "metrics-pipe.h" +#include "cfg-source.h" #include @@ -282,6 +283,14 @@ log_expr_node_new(gint layout, gint content, const gchar *name, LogExprNode *chi self->line = yylloc->first_line; self->column = yylloc->first_column; } + if (yylloc && (debug_flag || 1)) + { + CFG_LTYPE lloc = *yylloc; + lloc.last_column = 255; + GString *text = g_string_new(""); + cfg_source_extract_source_text(configuration->lexer, &lloc, text); + self->expr_text = g_string_free(text, FALSE); + } return self; } @@ -308,6 +317,7 @@ _log_expr_node_free(LogExprNode *self) self->aux_destroy(self->aux); g_free(self->name); g_free(self->filename); + g_free(self->expr_text); g_free(self); } diff --git a/lib/cfg-tree.h b/lib/cfg-tree.h index 4e446881d4..06117b5e14 100644 --- a/lib/cfg-tree.h +++ b/lib/cfg-tree.h @@ -124,6 +124,7 @@ struct _LogExprNode gchar *filename; gint line, column; gint child_id; + gchar *expr_text; }; gint log_expr_node_lookup_flag(const gchar *flag); diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index 1e039982ab..8530df7b4f 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -28,6 +28,7 @@ #include "mainloop.h" #include "stats/stats-registry.h" #include "stats/stats-cluster-single.h" +#include "perf/perf.h" void filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gchar *text) @@ -36,7 +37,7 @@ filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gc self->lloc = g_new0(CFG_LTYPE, 1); *self->lloc = *lloc; - if (debug_flag && text) + if ((debug_flag || 1) && text) self->expr_text = g_strdup(text); } @@ -46,7 +47,7 @@ filterx_expr_set_location(FilterXExpr *self, CfgLexer *lexer, CFG_LTYPE *lloc) if (!self->lloc) self->lloc = g_new0(CFG_LTYPE, 1); *self->lloc = *lloc; - if (debug_flag) + if (debug_flag || 1) { GString *res = g_string_sized_new(0); cfg_source_extract_source_text(lexer, lloc, res); @@ -110,7 +111,7 @@ _init_sc_key_name(FilterXExpr *self, gchar *buf, gsize buf_len) gboolean filterx_expr_init_method(FilterXExpr *self, GlobalConfig *cfg) { - gchar buf[64]; + gchar buf[256]; _init_sc_key_name(self, buf, sizeof(buf)); stats_lock(); @@ -123,6 +124,22 @@ filterx_expr_init_method(FilterXExpr *self, GlobalConfig *cfg) stats_cluster_single_key_set(&sc_key, buf, labels, labels_len); stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->eval_count); stats_unlock(); + + if (!perf_is_trampoline_address(self->eval)) + { + //g_snprintf(buf, sizeof(buf), "filterx_%s_eval(%s)", self->type, self->name ? : "anon"); + if (self->lloc) +// g_snprintf(buf, sizeof(buf), "filterx::%s:%d:%d|\t%s", +// self->lloc->name, self->lloc->first_line, self->lloc->first_column, +// self->expr_text ? : "n/a"); + g_snprintf(buf, sizeof(buf), "filterx::%s", self->expr_text ? : "n/a"); + else + g_snprintf(buf, sizeof(buf), "filterx::%s(%s)", self->type, self->name ? : "anon"); +// msg_error("installing trampoline on expr", +// evt_tag_str("expr", buf)); + self->eval = perf_generate_trampoline(self->eval, buf); + } + return TRUE; } diff --git a/lib/logmpx.c b/lib/logmpx.c index 6cd119756b..49a63b02a3 100644 --- a/lib/logmpx.c +++ b/lib/logmpx.c @@ -93,6 +93,11 @@ log_multiplexer_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_op * write protected so that changes in those branches don't overwrite * data we still need */ + msg_trace("logmpx: making message readonly", + evt_tag_int("next_hops", self->next_hops->len), + evt_tag_int("pipe_next", !!self->super.pipe_next), + log_expr_node_location_tag(self->super.expr_node), + evt_tag_str("expr_text", self->super.expr_node && self->super.expr_node->expr_text ? self->super.expr_node->expr_text : "n/a")); filterx_eval_prepare_for_fork(path_options->filterx_context, &msg, path_options); log_msg_write_protect(msg); } diff --git a/lib/logpipe.c b/lib/logpipe.c index 2a3fbdec69..1f0b470970 100644 --- a/lib/logpipe.c +++ b/lib/logpipe.c @@ -25,6 +25,7 @@ #include "logpipe.h" #include "cfg-tree.h" #include "cfg-walker.h" +#include "perf/perf.h" gboolean (*pipe_single_step_hook)(LogPipe *pipe, LogMessage *msg, const LogPathOptions *path_options); @@ -146,6 +147,20 @@ log_pipe_clone_method(LogPipe *dst, const LogPipe *src) log_pipe_set_options(dst, &src->options); } +static gboolean +log_pipe_post_config_init_method(LogPipe *self) +{ +// GlobalConfig *cfg = log_pipe_get_config(self); + if (self->flags & PIF_CONFIG_RELATED) + { + gchar buf[256]; + /* FIXME: check that perf profiling is enabled */ + if (!perf_is_trampoline_address(self->queue)) + self->queue = perf_generate_trampoline(self->queue, log_expr_node_format_location(self->expr_node, buf, sizeof(buf))); + } + return TRUE; +} + void log_pipe_init_instance(LogPipe *self, GlobalConfig *cfg) { @@ -154,7 +169,7 @@ log_pipe_init_instance(LogPipe *self, GlobalConfig *cfg) self->pipe_next = NULL; self->persist_name = NULL; self->plugin_name = NULL; - + self->post_config_init = log_pipe_post_config_init_method; self->queue = log_pipe_forward_msg; self->free_fn = log_pipe_free_method; self->arcs = _arcs; From 6521dce7b5947a7e26f9b83171650353ae44699f Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Wed, 1 Jan 2025 15:26:22 +0100 Subject: [PATCH 44/47] Revert "WIP: temporarily enable the extraction of source text" This reverts commit e93c530c12f03547552449c1fb52437efb9b86cb. --- lib/cfg-grammar.y | 4 ++-- lib/cfg-tree.c | 10 ---------- lib/cfg-tree.h | 1 - lib/filterx/filterx-expr.c | 23 +++-------------------- lib/logmpx.c | 5 ----- lib/logpipe.c | 17 +---------------- 6 files changed, 6 insertions(+), 54 deletions(-) diff --git a/lib/cfg-grammar.y b/lib/cfg-grammar.y index 87891bce3e..59decde075 100644 --- a/lib/cfg-grammar.y +++ b/lib/cfg-grammar.y @@ -567,14 +567,14 @@ expr_stmt source_stmt : KW_SOURCE string '{' source_content '}' { - $$ = log_expr_node_new_source($2, $4, &@$); + $$ = log_expr_node_new_source($2, $4, &@1); free($2); } ; dest_stmt : KW_DESTINATION string '{' dest_content '}' { - $$ = log_expr_node_new_destination($2, $4, &@$); + $$ = log_expr_node_new_destination($2, $4, &@1); free($2); } ; diff --git a/lib/cfg-tree.c b/lib/cfg-tree.c index 748993c044..d86b2ac635 100644 --- a/lib/cfg-tree.c +++ b/lib/cfg-tree.c @@ -26,7 +26,6 @@ #include "logmpx.h" #include "logpipe.h" #include "metrics-pipe.h" -#include "cfg-source.h" #include @@ -283,14 +282,6 @@ log_expr_node_new(gint layout, gint content, const gchar *name, LogExprNode *chi self->line = yylloc->first_line; self->column = yylloc->first_column; } - if (yylloc && (debug_flag || 1)) - { - CFG_LTYPE lloc = *yylloc; - lloc.last_column = 255; - GString *text = g_string_new(""); - cfg_source_extract_source_text(configuration->lexer, &lloc, text); - self->expr_text = g_string_free(text, FALSE); - } return self; } @@ -317,7 +308,6 @@ _log_expr_node_free(LogExprNode *self) self->aux_destroy(self->aux); g_free(self->name); g_free(self->filename); - g_free(self->expr_text); g_free(self); } diff --git a/lib/cfg-tree.h b/lib/cfg-tree.h index 06117b5e14..4e446881d4 100644 --- a/lib/cfg-tree.h +++ b/lib/cfg-tree.h @@ -124,7 +124,6 @@ struct _LogExprNode gchar *filename; gint line, column; gint child_id; - gchar *expr_text; }; gint log_expr_node_lookup_flag(const gchar *flag); diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index 8530df7b4f..1e039982ab 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -28,7 +28,6 @@ #include "mainloop.h" #include "stats/stats-registry.h" #include "stats/stats-cluster-single.h" -#include "perf/perf.h" void filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gchar *text) @@ -37,7 +36,7 @@ filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gc self->lloc = g_new0(CFG_LTYPE, 1); *self->lloc = *lloc; - if ((debug_flag || 1) && text) + if (debug_flag && text) self->expr_text = g_strdup(text); } @@ -47,7 +46,7 @@ filterx_expr_set_location(FilterXExpr *self, CfgLexer *lexer, CFG_LTYPE *lloc) if (!self->lloc) self->lloc = g_new0(CFG_LTYPE, 1); *self->lloc = *lloc; - if (debug_flag || 1) + if (debug_flag) { GString *res = g_string_sized_new(0); cfg_source_extract_source_text(lexer, lloc, res); @@ -111,7 +110,7 @@ _init_sc_key_name(FilterXExpr *self, gchar *buf, gsize buf_len) gboolean filterx_expr_init_method(FilterXExpr *self, GlobalConfig *cfg) { - gchar buf[256]; + gchar buf[64]; _init_sc_key_name(self, buf, sizeof(buf)); stats_lock(); @@ -124,22 +123,6 @@ filterx_expr_init_method(FilterXExpr *self, GlobalConfig *cfg) stats_cluster_single_key_set(&sc_key, buf, labels, labels_len); stats_register_counter(STATS_LEVEL3, &sc_key, SC_TYPE_SINGLE_VALUE, &self->eval_count); stats_unlock(); - - if (!perf_is_trampoline_address(self->eval)) - { - //g_snprintf(buf, sizeof(buf), "filterx_%s_eval(%s)", self->type, self->name ? : "anon"); - if (self->lloc) -// g_snprintf(buf, sizeof(buf), "filterx::%s:%d:%d|\t%s", -// self->lloc->name, self->lloc->first_line, self->lloc->first_column, -// self->expr_text ? : "n/a"); - g_snprintf(buf, sizeof(buf), "filterx::%s", self->expr_text ? : "n/a"); - else - g_snprintf(buf, sizeof(buf), "filterx::%s(%s)", self->type, self->name ? : "anon"); -// msg_error("installing trampoline on expr", -// evt_tag_str("expr", buf)); - self->eval = perf_generate_trampoline(self->eval, buf); - } - return TRUE; } diff --git a/lib/logmpx.c b/lib/logmpx.c index 49a63b02a3..6cd119756b 100644 --- a/lib/logmpx.c +++ b/lib/logmpx.c @@ -93,11 +93,6 @@ log_multiplexer_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_op * write protected so that changes in those branches don't overwrite * data we still need */ - msg_trace("logmpx: making message readonly", - evt_tag_int("next_hops", self->next_hops->len), - evt_tag_int("pipe_next", !!self->super.pipe_next), - log_expr_node_location_tag(self->super.expr_node), - evt_tag_str("expr_text", self->super.expr_node && self->super.expr_node->expr_text ? self->super.expr_node->expr_text : "n/a")); filterx_eval_prepare_for_fork(path_options->filterx_context, &msg, path_options); log_msg_write_protect(msg); } diff --git a/lib/logpipe.c b/lib/logpipe.c index 1f0b470970..2a3fbdec69 100644 --- a/lib/logpipe.c +++ b/lib/logpipe.c @@ -25,7 +25,6 @@ #include "logpipe.h" #include "cfg-tree.h" #include "cfg-walker.h" -#include "perf/perf.h" gboolean (*pipe_single_step_hook)(LogPipe *pipe, LogMessage *msg, const LogPathOptions *path_options); @@ -147,20 +146,6 @@ log_pipe_clone_method(LogPipe *dst, const LogPipe *src) log_pipe_set_options(dst, &src->options); } -static gboolean -log_pipe_post_config_init_method(LogPipe *self) -{ -// GlobalConfig *cfg = log_pipe_get_config(self); - if (self->flags & PIF_CONFIG_RELATED) - { - gchar buf[256]; - /* FIXME: check that perf profiling is enabled */ - if (!perf_is_trampoline_address(self->queue)) - self->queue = perf_generate_trampoline(self->queue, log_expr_node_format_location(self->expr_node, buf, sizeof(buf))); - } - return TRUE; -} - void log_pipe_init_instance(LogPipe *self, GlobalConfig *cfg) { @@ -169,7 +154,7 @@ log_pipe_init_instance(LogPipe *self, GlobalConfig *cfg) self->pipe_next = NULL; self->persist_name = NULL; self->plugin_name = NULL; - self->post_config_init = log_pipe_post_config_init_method; + self->queue = log_pipe_forward_msg; self->free_fn = log_pipe_free_method; self->arcs = _arcs; From aef340fda47574969c78f6a8d619263caf92b7f3 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 09:09:14 +0100 Subject: [PATCH 45/47] filterx: add filterx_expr_is_getattr/get_subscript() functions Signed-off-by: Balazs Scheidler --- lib/filterx/expr-get-subscript.c | 6 ++++++ lib/filterx/expr-get-subscript.h | 2 +- lib/filterx/expr-getattr.c | 6 ++++++ lib/filterx/expr-getattr.h | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/filterx/expr-get-subscript.c b/lib/filterx/expr-get-subscript.c index bf383318ae..4baabc80fc 100644 --- a/lib/filterx/expr-get-subscript.c +++ b/lib/filterx/expr-get-subscript.c @@ -169,3 +169,9 @@ filterx_get_subscript_new(FilterXExpr *operand, FilterXExpr *key) self->key = key; return &self->super; } + +gboolean +filterx_expr_is_get_subscript(FilterXExpr *s) +{ + return s->eval == _eval_get_subscript; +} diff --git a/lib/filterx/expr-get-subscript.h b/lib/filterx/expr-get-subscript.h index 751c29e211..2696a51a12 100644 --- a/lib/filterx/expr-get-subscript.h +++ b/lib/filterx/expr-get-subscript.h @@ -26,6 +26,6 @@ #include "filterx/filterx-expr.h" FilterXExpr *filterx_get_subscript_new(FilterXExpr *lhs, FilterXExpr *key); - +gboolean filterx_expr_is_get_subscript(FilterXExpr *s); #endif diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index f8c0bb77e9..c44d2fa535 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -151,3 +151,9 @@ filterx_getattr_new(FilterXExpr *operand, FilterXString *attr_name) self->super.name = filterx_string_get_value_ref(self->attr, NULL); return &self->super; } + +gboolean +filterx_expr_is_getattr(FilterXExpr *s) +{ + return s->eval == _eval_getattr; +} diff --git a/lib/filterx/expr-getattr.h b/lib/filterx/expr-getattr.h index 044b4fa723..20bf8f6179 100644 --- a/lib/filterx/expr-getattr.h +++ b/lib/filterx/expr-getattr.h @@ -27,6 +27,6 @@ #include "filterx/object-string.h" FilterXExpr *filterx_getattr_new(FilterXExpr *lhs, FilterXString *attr_name); - +gboolean filterx_expr_is_getattr(FilterXExpr *s); #endif From 830c56532eceb24d85a8ebd4a665255f64fa5d62 Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 11:53:14 +0100 Subject: [PATCH 46/47] filterx/filterx-expr: fix potential memory leak for expr_text In case an expr is optimized we might be setting the location of exprs multiple times, prepare for this case by freeing expr_text before setting it first. Signed-off-by: Balazs Scheidler --- lib/filterx/filterx-expr.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c index 1e039982ab..e16ab1e046 100644 --- a/lib/filterx/filterx-expr.c +++ b/lib/filterx/filterx-expr.c @@ -36,8 +36,11 @@ filterx_expr_set_location_with_text(FilterXExpr *self, CFG_LTYPE *lloc, const gc self->lloc = g_new0(CFG_LTYPE, 1); *self->lloc = *lloc; - if (debug_flag && text) - self->expr_text = g_strdup(text); + if (debug_flag && text && text != self->expr_text) + { + g_free(self->expr_text); + self->expr_text = g_strdup(text); + } } void @@ -48,6 +51,7 @@ filterx_expr_set_location(FilterXExpr *self, CfgLexer *lexer, CFG_LTYPE *lloc) *self->lloc = *lloc; if (debug_flag) { + g_free(self->expr_text); GString *res = g_string_sized_new(0); cfg_source_extract_source_text(lexer, lloc, res); self->expr_text = g_string_free(res, FALSE); From d940c5052d54f0294ea8e4e53277ce00e94933da Mon Sep 17 00:00:00 2001 From: Balazs Scheidler Date: Thu, 9 Jan 2025 11:53:31 +0100 Subject: [PATCH 47/47] merge: getattr opt --- lib/filterx/expr-getattr.c | 110 ++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 13 deletions(-) diff --git a/lib/filterx/expr-getattr.c b/lib/filterx/expr-getattr.c index c44d2fa535..bfe58c34bc 100644 --- a/lib/filterx/expr-getattr.c +++ b/lib/filterx/expr-getattr.c @@ -31,47 +31,74 @@ typedef struct _FilterXGetAttr FilterXExpr super; FilterXExpr *operand; FilterXObject *attr; + FilterXObject **path; } FilterXGetAttr; +static FilterXObject * +_eval_operand(FilterXGetAttr *self, FilterXObject **attr) +{ + FilterXObject *o = filterx_expr_eval_typed(self->operand); + + if (!o) + return NULL; + + *attr = self->attr; + if (*attr) + return o; + + FilterXObject *next_o; + *attr = self->path[0]; + for (gint i = 0; self->path[i] && self->path[i+1]; i++) + { + next_o = filterx_object_getattr(o, *attr); + if (!o) + break; + filterx_object_unref(o); + o = next_o; + *attr = self->path[i+1]; + } + return o; +} + static FilterXObject * _eval_getattr(FilterXExpr *s) { FilterXGetAttr *self = (FilterXGetAttr *) s; + FilterXObject *attr; - FilterXObject *variable = filterx_expr_eval_typed(self->operand); - + FilterXObject *variable = _eval_operand(self, &attr); if (!variable) return NULL; - FilterXObject *attr = filterx_object_getattr(variable, self->attr); - if (!attr) + FilterXObject *value = filterx_object_getattr(variable, attr); + if (!value) { - filterx_eval_push_error("Attribute lookup failed", s, self->attr); + filterx_eval_push_error("Attribute lookup failed", s, attr); goto exit; } exit: filterx_object_unref(variable); - return attr; + return value; } static gboolean _unset(FilterXExpr *s) { FilterXGetAttr *self = (FilterXGetAttr *) s; - + FilterXObject *attr; gboolean result = FALSE; - FilterXObject *variable = filterx_expr_eval_typed(self->operand); + FilterXObject *variable = _eval_operand(self, &attr); if (!variable) return FALSE; if (variable->readonly) { - filterx_eval_push_error("Object unset-attr failed, object is readonly", s, self->attr); + filterx_eval_push_error("Object unset-attr failed, object is readonly", s, attr); goto exit; } - result = filterx_object_unset_key(variable, self->attr); + result = filterx_object_unset_key(variable, attr); exit: filterx_object_unref(variable); @@ -82,22 +109,66 @@ static gboolean _isset(FilterXExpr *s) { FilterXGetAttr *self = (FilterXGetAttr *) s; - FilterXObject *variable = filterx_expr_eval_typed(self->operand); + FilterXObject *attr; + FilterXObject *variable = _eval_operand(self, &attr); if (!variable) return FALSE; - gboolean result = filterx_object_is_key_set(variable, self->attr); + gboolean result = filterx_object_is_key_set(variable, attr); filterx_object_unref(variable); return result; } +static void +_rollup_child_getattr(FilterXGetAttr *self, FilterXGetAttr *child) +{ + /* turn a list of getattrs into a single getattr with a path argument */ + GPtrArray *path = g_ptr_array_new(); + + if (child->attr) + { + /* if this is an unoptimized FilterXGetAttr, let's use the "attr" member */ + g_ptr_array_add(path, filterx_object_ref(child->attr)); + g_assert(child->path == NULL); + } + else + { + /* optimized GetAttr, copy the entire path into ours */ + for (gint i = 0; child->path[i]; i++) + { + g_ptr_array_add(path, filterx_object_ref(child->path[i])); + } + } + + /* we are the last getattr so append it to the end */ + g_ptr_array_add(path, self->attr); + /* null termination */ + g_ptr_array_add(path, NULL); + + /* replace self->attr with self->path */ + self->attr = NULL; + self->path = (FilterXObject **) g_ptr_array_free(path, FALSE); +} + static FilterXExpr * _optimize(FilterXExpr *s) { FilterXGetAttr *self = (FilterXGetAttr *) s; self->operand = filterx_expr_optimize(self->operand); + if (filterx_expr_is_getattr(self->operand)) + { + FilterXGetAttr *child = (FilterXGetAttr *) self->operand; + + _rollup_child_getattr(self, child); + self->operand = filterx_expr_ref(child->operand); + + filterx_expr_unref(&child->super); + + /* we need to return a ref */ + return filterx_expr_ref(s); + } return NULL; } @@ -125,7 +196,20 @@ static void _free(FilterXExpr *s) { FilterXGetAttr *self = (FilterXGetAttr *) s; - filterx_object_unref(self->attr); + + if (self->attr) + { + filterx_object_unref(self->attr); + g_assert(self->path == NULL); + } + else + { + for (gint i = 0; self->path[i]; i++) + filterx_object_unref(self->path[i]); + g_free(self->path); + g_assert(self->attr == NULL); + } + filterx_expr_unref(self->operand); filterx_expr_free_method(s); }