@cryptotaxi247 / netdata-1 / commits / 0620e5c29

fix hardcoding of eval variables (#20210)

Costa Tsaousis committed Apr 30, 2025 at 13:52 UTC 0620e5c2998093d7ec5f497760e5d29944d64e85
4 files changed +327 -43
CMakeLists.txt
+1
@@ -852,6 +852,7 @@ set(LIBNETDATA_FILES
852 src/libnetdata/eval/eval.h
853 src/libnetdata/eval/eval-internal.h
854 src/libnetdata/eval/eval-unittest.c
855 + src/libnetdata/eval/eval-unittest-hardcoding.c
856 src/libnetdata/eval/re2c_lemon/lexer.c
857 src/libnetdata/eval/re2c_lemon/parser.c
858 src/libnetdata/eval/re2c_lemon/parser_wrapper.c
src/libnetdata/eval/eval-unittest-hardcoding.c new
+244
@@ -0,0 +1,244 @@
1 +// SPDX-License-Identifier: GPL-3.0-or-later
2 +
3 +#include "libnetdata/libnetdata.h"
4 +#include "eval-internal.h"
5 +
6 +// Test case structure for hardcode variable tests
7 +typedef struct {
8 + const char *name; // Name/description of the test case
9 + const char *expression; // Initial expression to parse
10 + const char *variable; // Variable name to hardcode (NULL for testing NULL variable)
11 + NETDATA_DOUBLE hardcode_value; // Value to hardcode (NAN for testing NaN)
12 + const char *expected_source; // Expected expression source after hardcoding
13 + NETDATA_DOUBLE expected_result; // Expected result after evaluation
14 + int expected_error; // Expected error code (EVAL_ERROR_OK if no error)
15 +} HardcodeTestCase;
16 +
17 +int eval_hardcode_unittest(void) {
18 + printf("\n=== Running Tests for expression_hardcode_variable() ===\n");
19 +
20 + // Define all test cases as data
21 + HardcodeTestCase test_cases[] = {
22 + // Basic variable replacement
23 + {
24 + "Basic variable",
25 + "$test_var + 10",
26 + "test_var",
27 + 42.0,
28 + "42 + 10",
29 + 52.0,
30 + EVAL_ERROR_OK
31 + },
32 + // Variable with braces
33 + {
34 + "Variable with braces",
35 + "${test_var} * 2",
36 + "test_var",
37 + 42.0,
38 + "42 * 2",
39 + 84.0,
40 + EVAL_ERROR_OK
41 + },
42 + // Multiple occurrences of same variable
43 + {
44 + "Multiple occurrences",
45 + "$test_var + ${test_var} + $test_var + ${test_var} + $test_var + ${test_var}",
46 + "test_var",
47 + 42.0,
48 + "42 + 42 + 42 + 42 + 42 + 42",
49 + 252.0,
50 + EVAL_ERROR_OK
51 + },
52 + // Complex nested expression
53 + {
54 + "Complex expression",
55 + "($test_var > 30) ? (${test_var} * 2) : ($test_var / 2)",
56 + "test_var",
57 + 42.0,
58 + "(42 > 30) ? (42 * 2) : (42 / 2)",
59 + 84.0,
60 + EVAL_ERROR_OK
61 + },
62 + // Variable not in expression (should remain unchanged)
63 + {
64 + "Variable not in expression",
65 + "33 + 33",
66 + "test_var",
67 + 42.0,
68 + "33 + 33",
69 + 66.0,
70 + EVAL_ERROR_OK
71 + },
72 + // Hardcoding negative value
73 + {
74 + "Negative value",
75 + "$test_var * 10",
76 + "test_var",
77 + -5.0,
78 + "-5 * 10",
79 + -50.0,
80 + EVAL_ERROR_OK
81 + },
82 + // Hardcoding decimal value
83 + {
84 + "Decimal value",
85 + "$test_var / 10",
86 + "test_var",
87 + 123.456,
88 + "123.456 / 10",
89 + 12.3456,
90 + EVAL_ERROR_OK
91 + },
92 + // Function parameter
93 + {
94 + "Function parameter",
95 + "abs($test_var)",
96 + "test_var",
97 + -42.0,
98 + "abs(-42)",
99 + 42.0,
100 + EVAL_ERROR_OK
101 + },
102 + // NaN value
103 + {
104 + "NaN value (ignored)",
105 + "$test_var + 10",
106 + "test_var",
107 + NAN,
108 + "nan + 10", // source should remain unchanged
109 + 0.0, // Unresolved variable error
110 + EVAL_ERROR_VALUE_IS_NAN
111 + },
112 + // INFINITY value
113 + {
114 + "NaN value (ignored)",
115 + "$test_var + 10",
116 + "test_var",
117 + INFINITY,
118 + "inf + 10", // source should remain unchanged
119 + 0.0, // Unresolved variable error
120 + EVAL_ERROR_VALUE_IS_INFINITE
121 + },
122 + // NULL expression (no crash test)
123 + {
124 + "NULL expression",
125 + NULL, // This isn't actually used - we'll handle it specially
126 + "test_var",
127 + 42.0,
128 + NULL, // Expected source doesn't matter
129 + 0.0, // Result doesn't matter
130 + EVAL_ERROR_OK
131 + },
132 + // NULL variable (no crash test)
133 + {
134 + "NULL variable",
135 + "$test_var + 10",
136 + NULL, // NULL variable name
137 + 42.0,
138 + "$test_var + 10", // source should remain unchanged
139 + 0.0, // Unresolved variable error
140 + EVAL_ERROR_UNKNOWN_VARIABLE
141 + }
142 + };
143 +
144 + int passed = 0;
145 + int failed = 0;
146 +
147 + // Single loop to process all test cases
148 + for (size_t i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) {
149 + HardcodeTestCase *tc = &test_cases[i];
150 +
151 + printf("Test %zu: %s\n", i + 1, tc->name);
152 +
153 + // Handle special case of NULL expression test (just don't crash)
154 + if (!tc->expression) {
155 + printf(" Testing NULL expression (shouldn't crash)...\n");
156 + STRING *var = tc->variable ? string_strdupz(tc->variable) : NULL;
157 +
158 + // This call shouldn't crash
159 + expression_hardcode_variable(NULL, var, tc->hardcode_value);
160 +
161 + printf(" PASSED: No crash with NULL expression\n");
162 + passed++;
163 + if (var) string_freez(var);
164 + continue;
165 + }
166 +
167 + // Parse the expression
168 + const char *failed_at = NULL;
169 + int error = 0;
170 + EVAL_EXPRESSION *exp = expression_parse(tc->expression, &failed_at, &error);
171 +
172 + if (!exp) {
173 + printf(" FAILED: Could not parse expression, error: %d (%s)\n",
174 + error, expression_strerror(error));
175 + failed++;
176 + continue;
177 + }
178 +
179 + // Save the original source
180 + const char *original_source = expression_source(exp);
181 + printf(" Original source: %s\n", original_source);
182 +
183 + // Hardcode the variable
184 + STRING *var = tc->variable ? string_strdupz(tc->variable) : NULL;
185 + expression_hardcode_variable(exp, var, tc->hardcode_value);
186 + if (var) string_freez(var);
187 +
188 + // Get the modified source
189 + const char *modified_source = expression_source(exp);
190 + printf(" Modified source: %s\n", modified_source);
191 +
192 + // Check if source was modified as expected
193 + bool source_correct = true;
194 + if (tc->expected_source &&
195 + strcmp(modified_source, tc->expected_source) != 0) {
196 + printf(" FAILED: Source doesn't match expected.\n");
197 + printf(" Expected: %s\n", tc->expected_source);
198 + printf(" Actual: %s\n", modified_source);
199 + source_correct = false;
200 + }
201 +
202 + // Evaluate the expression
203 + expression_evaluate(exp);
204 +
205 + // Check error code
206 + bool error_correct = (exp->error == tc->expected_error);
207 + if (!error_correct) {
208 + printf(" FAILED: Error code doesn't match expected.\n");
209 + printf(" Expected error: %d (%s)\n",
210 + tc->expected_error, expression_strerror(tc->expected_error));
211 + printf(" Actual error: %d (%s)\n",
212 + exp->error, expression_strerror(exp->error));
213 + }
214 +
215 + // Check result if we expected no error
216 + bool result_correct = true;
217 + if (tc->expected_error == EVAL_ERROR_OK) {
218 + NETDATA_DOUBLE result = expression_result(exp);
219 + printf(" Result: %f\n", result);
220 +
221 + if (fabs(result - tc->expected_result) > 0.000001) {
222 + printf(" FAILED: Result doesn't match expected.\n");
223 + printf(" Expected: %f\n", tc->expected_result);
224 + printf(" Actual: %f\n", result);
225 + result_correct = false;
226 + }
227 + }
228 +
229 + // Determine if test passed overall
230 + if (source_correct && error_correct && result_correct) {
231 + printf(" PASSED\n");
232 + passed++;
233 + } else {
234 + failed++;
235 + }
236 +
237 + // Clean up
238 + expression_free(exp);
239 + }
240 +
241 + // Report results
242 + printf("\nHardcode variable test results: %d passed, %d failed\n", passed, failed);
243 + return failed;
244 +}
\ No newline at end of file
src/libnetdata/eval/eval-unittest.c
+6 -1
@@ -1110,6 +1110,8 @@ static TestGroup test_groups[] = {
1110 {"Crash Tests", crash_tests, ARRAY_SIZE(crash_tests)},
1111 };
1112
1113 +int eval_hardcode_unittest(void);
1114 +
1115 int eval_unittest(void) {
1116 // Test cases for basic arithmetic operations
1117
@@ -1203,6 +1205,9 @@ int eval_unittest(void) {
1205 printf("Total tests: %d\n", total_tests);
1206 printf("Passed: %d (%.1f%%)\n", total_passed, (float)total_passed / total_tests * 100);
1207 printf("Failed: %d (%.1f%%)\n", total_failed, (float)total_failed / total_tests * 100);
1206 -
1208 +
1209 + if(!total_failed)
1210 + return eval_hardcode_unittest();
1211 +
1212 return total_failed > 0 ? 1 : 0;
1213 }
src/libnetdata/eval/eval-utils.c
+76 -42
@@ -292,21 +292,65 @@ static size_t expression_hardcode_node_variable(EVAL_NODE *node, STRING *variabl
292 return matches;
293 }
294
295 +static size_t str_replace_cpy(char *dst, size_t dst_size,
296 + const char *src,
297 + const char *variable, size_t variable_len,
298 + const char *value, size_t value_len) {
299 +
300 + if (!dst || !src || !variable || !value || dst_size == 0 || variable_len == 0 || value_len == 0)
301 + return 0;
302 +
303 + const char *pos = strstr(src, variable);
304 + if (!pos)
305 + return 0;
306 +
307 + size_t src_idx = 0;
308 + size_t dst_idx = 0;
309 + size_t matches = 0;
310 +
311 + while (src[src_idx] != '\0') {
312 + if (pos && &src[src_idx] == pos) {
313 + if (dst_idx + value_len >= dst_size)
314 + return 0;
315 +
316 + matches++;
317 + memcpy(&dst[dst_idx], value, value_len);
318 + dst_idx += value_len;
319 + src_idx += variable_len;
320 +
321 + pos = strstr(&src[src_idx], variable);
322 + } else {
323 + if (dst_idx + 1 >= dst_size)
324 + return 0;
325 + dst[dst_idx++] = src[src_idx++];
326 + }
327 + }
328 +
329 + if (dst_idx >= dst_size)
330 + return 0;
331 +
332 + dst[dst_idx] = '\0';
333 +
334 + return matches;
335 +}
336 +
337 +
338 void expression_hardcode_variable(EVAL_EXPRESSION *expression, STRING *variable, NETDATA_DOUBLE value) {
296 - if (!expression || !variable || isnan(value))
339 + if (!expression || !variable)
340 return;
341
342 size_t matches = expression_hardcode_node_variable(expression->nodes, variable, value);
343 if (matches) {
301 - char replace[1024];
302 - snprintfz(replace, sizeof(replace), NETDATA_DOUBLE_FORMAT_AUTO, value);
344 + char replace[DOUBLE_MAX_LENGTH];
345 + if(isnan(value))
346 + strncpyz(replace, "nan", sizeof(replace));
347 + else if(isinf(value))
348 + strncpyz(replace, "inf", sizeof(replace));
349 + else
350 + print_netdata_double(replace, value);
351 size_t replace_len = strlen(replace);
352
353 size_t source_len = string_strlen(expression->source);
306 - const char *source_str = string2str(expression->source);
307 -
308 - // Allocate enough space to accommodate all replacements.
309 - char buf[source_len + 1 + matches * (replace_len + 1)];
354
355 char find1[string_strlen(variable) + 1 + 1];
356 snprintfz(find1, sizeof(find1), "$%s", string2str(variable));
@@ -316,50 +360,40 @@ void expression_hardcode_variable(EVAL_EXPRESSION *expression, STRING *variable,
360 snprintfz(find2, sizeof(find2), "${%s}", string2str(variable));
361 size_t find2_len = strlen(find2);
362
319 - size_t found = 0; (void)found;
320 - char *buf_ptr = buf;
321 - const char *source_ptr = source_str;
363 + // Calculate the maximum possible buffer size needed
364 + // Source length + (max replacement length - min variable length) * matches + null terminator
365 + size_t min_var_len = MIN(find1_len, find2_len);
366 + size_t max_buf_size = source_len + 1 + (matches * (replace_len > min_var_len ? replace_len - min_var_len : 0));
367
323 - while (*source_ptr) {
324 - char *s1 = strstr(source_ptr, find1);
325 - char *s2 = strstr(source_ptr, find2);
368 + char buf1[max_buf_size];
369 + char buf2[max_buf_size];
370
327 - char *s = s1;
328 - size_t len = find1_len;
329 - if (s2 && (!s1 || s2 < s1)) {
330 - s = s2;
331 - len = find2_len;
371 + char *dst[2] = {buf1, buf2};
372 +
373 + const char *src = string2str(expression->source);
374 + size_t slot = 0;
375 + while(matches) {
376 + size_t matched = 0;
377 +
378 + matched = str_replace_cpy(dst[slot], max_buf_size, src, find1, find1_len, replace, replace_len);
379 + if(matched) {
380 + src = dst[slot];
381 + if(++slot > 1) slot = 0;
382 + matches -= MIN(matches, matched);
383 }
384
334 - if (s) {
335 - // Skip this check since the function has been moved to eval-parser.c
336 - if (s == s1) {
337 - // Move past the variable if it's part of a larger word.
338 - source_ptr = s + len;
339 - continue;
385 + if(matches) {
386 + matched = str_replace_cpy(dst[slot], max_buf_size, src, find2, find2_len, replace, replace_len);
387 + if(matched) {
388 + src = dst[slot];
389 + if (++slot > 1) slot = 0;
390 + matches -= MIN(matches, matched);
391 }
341 -
342 - // Copy the part before the variable.
343 - memcpy(buf_ptr, source_ptr, s - source_ptr);
344 - buf_ptr += (s - source_ptr);
345 -
346 - // Copy the replacement.
347 - memcpy(buf_ptr, replace, replace_len);
348 - buf_ptr += replace_len;
349 - *buf_ptr = '\0';
350 -
351 - // Move the source pointer past the replaced variable.
352 - source_ptr = s + len;
353 - found++;
354 - } else {
355 - // Copy the rest of the string if no more variables are found.
356 - strcpy(buf_ptr, source_ptr);
357 - break;
392 }
393 }
394
395 // Update the expression source with the new string.
396 string_freez(expression->source);
363 - expression->source = string_strdupz(buf);
397 + expression->source = string_strdupz(src);
398 }
399 }