filterx: let assign/setattr/set-subscript return the rhs

Instead of always returning TRUE to indicate success, return the rhs and introduce an "ignore_falsy_return" property on the expression to indicate that a falsy return is to be ignored. NULL returns are still considered errors. This removes the need for custom trace messages to find the results of assignments. Signed-off-by: Balazs Scheidler <[email protected]>
bazsi · May 6, 2024 · 2cab8b7 · 2cab8b7
1 parent 6b7b015
commit 2cab8b7
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 73 deletions.
diff --git a/lib/filterx/expr-assign.c b/lib/filterx/expr-assign.c
@@ -30,29 +30,17 @@ _eval(FilterXExpr *s)
   FilterXBinaryOp *self = (FilterXBinaryOp *) s;
 
   FilterXObject *value = filterx_expr_eval(self->rhs);
-  FilterXObject *result = NULL;
+
   if (!value)
     return NULL;
 
-  if (filterx_expr_assign(self->lhs, value))
+  if (!filterx_expr_assign(self->lhs, value))
     {
-      result = filterx_boolean_new(TRUE);
-      if (trace_flag)
-        {
-          GString *buf = scratch_buffers_alloc();
-          if (value && !filterx_object_repr(value, buf))
-            {
-              LogMessageValueType t;
-              if (!filterx_object_marshal(value, buf, &t))
-                g_assert_not_reached();
-            }
-          msg_trace("Filterx assignment",
-                    evt_tag_mem("value", buf->str, buf->len));
-        }
+      filterx_object_unref(value);
+      return NULL;
     }
 
-  filterx_object_unref(value);
-  return result;
+  return value;
 }
 
 /* NOTE: takes the object reference */
@@ -63,5 +51,6 @@ filterx_assign_new(FilterXExpr *lhs, FilterXExpr *rhs)
 
   filterx_binary_op_init_instance(self, lhs, rhs);
   self->super.eval = _eval;
+  self->super.ignore_falsy_result = TRUE;
   return &self->super;
 }
diff --git a/lib/filterx/expr-set-subscript.c b/lib/filterx/expr-set-subscript.c
@@ -69,43 +69,16 @@ _eval(FilterXExpr *s)
   FilterXObject *cloned = filterx_object_clone(new_value);
   filterx_object_unref(new_value);
 
-  if (filterx_object_set_subscript(object, key, &cloned))
+  if (!filterx_object_set_subscript(object, key, &cloned))
     {
-      result = filterx_boolean_new(TRUE);
-      if (trace_flag)
-        {
-          GString *buf = scratch_buffers_alloc();
-          if (cloned && !filterx_object_repr(cloned, buf))
-            {
-              LogMessageValueType t;
-              if (!filterx_object_marshal(cloned, buf, &t))
-                g_assert_not_reached();
-            }
-
-          GString *key_buf = scratch_buffers_alloc();
-          if (!key)
-            {
-              g_string_assign(key_buf, "(null)");
-            }
-          else if (!filterx_object_repr(key, buf))
-            {
-              LogMessageValueType t;
-              if (!filterx_object_marshal(key, buf, &t))
-                g_assert_not_reached();
-            }
-
-          msg_trace("Filterx set-subscript",
-                    evt_tag_mem("key", key_buf->str, key_buf->len),
-                    evt_tag_mem("value", buf->str, buf->len));
-        }
+      filterx_eval_push_error("Object set-subscript failed", s, key);
+      filterx_object_unref(cloned);
     }
   else
     {
-      filterx_eval_push_error("Object set-subscript failed", s, key);
+      result = cloned;
     }
 
-  filterx_object_unref(cloned);
-
 exit:
   filterx_object_unref(key);
   filterx_object_unref(object);
@@ -134,5 +107,6 @@ filterx_set_subscript_new(FilterXExpr *object, FilterXExpr *key, FilterXExpr *ne
   self->object = object;
   self->key = key;
   self->new_value = new_value;
+  self->super.ignore_falsy_result = TRUE;
   return &self->super;
 }
diff --git a/lib/filterx/expr-setattr.c b/lib/filterx/expr-setattr.c
@@ -57,30 +57,16 @@ _eval(FilterXExpr *s)
   FilterXObject *cloned = filterx_object_clone(new_value);
   filterx_object_unref(new_value);
 
-  if (filterx_object_setattr(object, self->attr, &cloned))
+  if (!filterx_object_setattr(object, self->attr, &cloned))
     {
-      result = filterx_boolean_new(TRUE);
-      if (trace_flag)
-        {
-          GString *buf = scratch_buffers_alloc();
-          if (cloned && !filterx_object_repr(cloned, buf))
-            {
-              LogMessageValueType t;
-              if (!filterx_object_marshal(cloned, buf, &t))
-                g_assert_not_reached();
-            }
-          msg_trace("Filterx setattr",
-                    evt_tag_str("attr", filterx_string_get_value(self->attr, NULL)),
-                    evt_tag_mem("value", buf->str, buf->len));
-        }
+      filterx_eval_push_error("Attribute set failed", s, self->attr);
+      filterx_object_unref(cloned);
     }
   else
     {
-      filterx_eval_push_error("Attribute set failed", s, self->attr);
+      result = cloned;
     }
 
-  filterx_object_unref(cloned);
-
 exit:
   filterx_object_unref(object);
   return result;
@@ -109,5 +95,6 @@ filterx_setattr_new(FilterXExpr *object, const gchar *attr_name, FilterXExpr *ne
   self->object = object;
   self->attr = filterx_string_new(attr_name, -1);
   self->new_value = new_value;
+  self->super.ignore_falsy_result = TRUE;
   return &self->super;
 }
diff --git a/lib/filterx/filterx-eval.c b/lib/filterx/filterx-eval.c
@@ -120,14 +120,14 @@ _evaluate_statement(FilterXExpr *expr)
 
   if (!res)
     {
-      msg_debug("FILTERX  ERROR",
+      msg_debug("FILTERX ERROR",
                 filterx_expr_format_location_tag(expr),
                 filterx_format_last_error());
       return FALSE;
     }
   filterx_eval_clear_errors();
 
-  success = filterx_object_truthy(res);
+  success = expr->ignore_falsy_result || filterx_object_truthy(res);
   if (!success || trace_flag)
     {
       GString *buf = scratch_buffers_alloc();
@@ -140,14 +140,15 @@ _evaluate_statement(FilterXExpr *expr)
         }
 
       if (!success)
-        msg_debug("FILTERX  FALSY",
+        msg_debug("FILTERX FALSY",
                   filterx_expr_format_location_tag(expr),
                   evt_tag_mem("value", buf->str, buf->len),
                   evt_tag_str("type", res->type->name));
       else
-        msg_trace("FILTERX TRUTHY",
+        msg_trace("FILTERX ESTEP",
                   filterx_expr_format_location_tag(expr),
                   evt_tag_mem("value", buf->str, buf->len),
+                  evt_tag_int("truthy", filterx_object_truthy(res)),
                   evt_tag_str("type", res->type->name));
     }
 

diff --git a/lib/filterx/filterx-expr.c b/lib/filterx/filterx-expr.c
@@ -136,7 +136,8 @@ filterx_expr_list_eval(GList *expressions, FilterXObject **result)
       FilterXExpr *expr = elem->data;
       *result = filterx_expr_eval(expr);
 
-      if (!(*result) || filterx_object_falsy(*result))
+      if (!(*result) ||
+          (!expr->ignore_falsy_result && filterx_object_falsy(*result)))
         return FALSE;
     }
 

diff --git a/lib/filterx/filterx-expr.h b/lib/filterx/filterx-expr.h
@@ -33,6 +33,7 @@ struct _FilterXExpr
 {
   guint32 ref_cnt;
   const gchar *type;
+  guint32 ignore_falsy_result:1;
 
   /* evaluate expression */
   FilterXObject *(*eval)(FilterXExpr *self);

diff --git a/lib/filterx/tests/test_filterx_expr.c b/lib/filterx/tests/test_filterx_expr.c
@@ -379,8 +379,10 @@ Test(filterx_expr, test_filterx_assign)
 
   FilterXObject *res = filterx_expr_eval(assign);
   cr_assert_not_null(res);
-  cr_assert(filterx_object_is_type(res, &FILTERX_TYPE_NAME(boolean)));
+  cr_assert(filterx_object_is_type(res, &FILTERX_TYPE_NAME(string)));
+  cr_assert_str_eq(filterx_string_get_value(res, NULL), "foobar");
   cr_assert(filterx_object_truthy(res));
+  cr_assert(assign->ignore_falsy_result);
 
   cr_assert_not_null(result_var);
   FilterXObject *result_obj = filterx_expr_eval(result_var);
@@ -419,8 +421,47 @@ Test(filterx_expr, test_filterx_setattr)
 
   FilterXObject *res = filterx_expr_eval(setattr);
   cr_assert_not_null(res);
-  cr_assert(filterx_object_is_type(res, &FILTERX_TYPE_NAME(boolean)));
+  cr_assert(filterx_object_is_type(res, &FILTERX_TYPE_NAME(string)));
+  cr_assert_str_eq(filterx_string_get_value(res, NULL), "bar");
   cr_assert(filterx_object_truthy(res));
+  cr_assert(setattr->ignore_falsy_result);
+
+  assert_object_json_equals(json, "{\"foo\":\"bar\"}");
+
+  filterx_expr_unref(setattr);
+  log_msg_unref(msg);
+  filterx_scope_unref(scope);
+  filterx_eval_set_context(NULL);
+}
+
+Test(filterx_expr, test_filterx_set_subscript)
+{
+  LogMessage *msg = create_sample_message();
+  FilterXScope *scope = filterx_scope_new();
+
+  FilterXEvalContext context =
+  {
+    .msgs = &msg,
+    .num_msg = 1,
+    .template_eval_options = &DEFAULT_TEMPLATE_EVAL_OPTIONS,
+    .scope = scope,
+  };
+  filterx_eval_set_context(&context);
+
+  FilterXObject *json = filterx_json_object_new_empty();
+  FilterXExpr *fillable = filterx_literal_new(json);
+
+  FilterXExpr *setattr = filterx_set_subscript_new(fillable,
+                                                   filterx_literal_new(filterx_string_new("foo", -1)),
+                                                   filterx_literal_new(filterx_string_new("bar", -1)));
+  cr_assert_not_null(setattr);
+
+  FilterXObject *res = filterx_expr_eval(setattr);
+  cr_assert_not_null(res);
+  cr_assert(filterx_object_is_type(res, &FILTERX_TYPE_NAME(string)));
+  cr_assert_str_eq(filterx_string_get_value(res, NULL), "bar");
+  cr_assert(filterx_object_truthy(res));
+  cr_assert(setattr->ignore_falsy_result);
 
   assert_object_json_equals(json, "{\"foo\":\"bar\"}");