config_format: fix caller-owned parse failure cleanup

[edsiper]

Summary

This fixes a config-format ownership bug found while reviewing PR #11939.
When flb_cf_fluentbit_create() or flb_cf_yaml_create() receives a caller-owned struct flb_cf * and parsing fails, the constructor must not destroy that object before returning NULL.

The constructors now only destroy config objects they allocate internally. Caller-owned configs remain owned by the caller on failure. The reload path also keeps the parse result in a temporary pointer so a failed reload can destroy the caller-owned new_cf and temporary context instead of leaking them.

Tests

• cmake --build build -j8
• ctest --test-dir build -R 'flb-it-(config_format_fluentbit|config_format_yaml|reload)' --output-on-failure
• valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=all ./build/bin/flb-it-config_format_fluentbit
• valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=all ./build/bin/flb-it-config_format_yaml
• valgrind --error-exitcode=1 --leak-check=full --show-leak-kinds=all ./build/bin/flb-it-reload --exec=never reload_yaml_missing_include
• git diff --check origin/master..HEAD
• /tmp/flb-commit-lint-11939/bin/python .github/scripts/commit_prefix_check.py
• GITHUB_EVENT_NAME=pull_request GITHUB_BASE_REF=master /tmp/flb-commit-lint-11939/bin/python .github/scripts/commit_prefix_check.py

Summary by CodeRabbit

• Bug Fixes

  • Improved cleanup and error handling when configuration loading fails (including missing include scenarios), preventing incorrect destruction behavior for caller-owned configuration objects.
  • Enhanced hot-reload behavior: if a reload config load fails, the reload is halted with complete cleanup.

• Tests

  • Added new fixtures and unit tests for classic and YAML “missing include” cases, including validation that failures are handled correctly for caller-owned configuration objects.
  • Added a hot-reload YAML test covering missing include behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d12bd2b5-0731-44be-b4d8-f5f4eba97c16

📥 Commits

Reviewing files that changed from the base of the PR and between e9ffa6a and 2bf4ac4.

📒 Files selected for processing (9)

• src/config_format/flb_cf_fluentbit.c
• src/config_format/flb_cf_yaml.c
• src/flb_reload.c
• tests/internal/config_format_fluentbit.c
• tests/internal/config_format_yaml.c
• tests/internal/data/config_format/classic/missing_include.conf
• tests/internal/data/config_format/yaml/missing_include.yaml
• tests/internal/data/reload/yaml/missing_include.yaml
• tests/internal/reload.c

✅ Files skipped from review due to trivial changes (3)

• tests/internal/data/config_format/classic/missing_include.conf
• tests/internal/data/config_format/yaml/missing_include.yaml
• tests/internal/data/reload/yaml/missing_include.yaml

🚧 Files skipped from review as they are similar to previous changes (2)

• src/config_format/flb_cf_yaml.c
• tests/internal/config_format_yaml.c

---

📝 Walkthrough

Walkthrough

flb_cf_fluentbit_create() and flb_cf_yaml_create() each gain a boolean ownership flag set only when they allocate the config object internally; error paths now destroy the config object only if that flag is set, preserving caller-provided objects. flb_reload() improves error handling during config loading by performing complete cleanup before halting. New test fixtures and test functions verify both the ownership preservation and reload behavior with missing-include scenarios.

Changes

Caller-owned cleanup fix and hot-reload improvements

Layer / File(s)	Summary
Ownership tracking in config create functions src/config_format/flb_cf_fluentbit.c, src/config_format/flb_cf_yaml.c	Both flb_cf_fluentbit_create() and flb_cf_yaml_create() gain a local flag (cf_created / conf_created) set to FLB_TRUE only when they allocate the config object. On local_init() or read_config() failure, the config object is destroyed only when that flag is true, leaving caller-provided objects untouched.
Hot-reload config loading error handling src/flb_reload.c	Introduces loaded_cf variable to track the config-format instance created from file; on load failure, destroys file, current new_cf, and new_ctx before halting reload cleanly with FLB_RELOAD_HALTED.
Missing-include test data fixtures tests/internal/data/config_format/classic/missing_include.conf, tests/internal/data/config_format/yaml/missing_include.yaml, tests/internal/data/reload/yaml/missing_include.yaml	New fixture files that reference nonexistent include targets, triggering predictable parse failures used by the new tests.
Config format tests for caller-owned objects tests/internal/config_format_fluentbit.c, tests/internal/config_format_yaml.c	Adds FLB_006 / FLB_007 path macros; introduces test_caller_owned_error() functions that create caller-owned config objects, pass them to the create functions, and verify they are not destroyed on failure; updates indent_level_error() and recursion() to use intermediate ret pointers with conditional cleanup; all tests registered in TEST_LIST.
Hot-reload test for missing-include scenario tests/internal/reload.c	Adds FLB_YAML_MISSING_INCLUDE macro and test_reload_yaml_missing_include() which starts the engine, switches to a YAML config with a missing include, triggers reload, and asserts the reload halts cleanly with FLB_RELOAD_HALTED; test registered in TEST_LIST.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• fluent/fluent-bit#10869: Modifies hot-reload control flow in src/flb_reload.c around resource cleanup and watchdog handling during reload operations.

Suggested reviewers

• cosmo0920

Poem

🐇 A rabbit once handed her carrot of choice,
But the config creator destroyed it—"no voice!"
Now a flag marks what's mine versus yours,
No more double-frees crashing through doors.
Tests confirm: your conf stays whole and secure,
Ownership handled—that's a bunny-approved cure! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: fixing cleanup behavior when caller-owned configuration objects fail to parse.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch yaml-include-cwd-11939

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9ffa6ace9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep a pointer to caller-owned cf on failure

When parsing fails for a caller-owned config, this path now returns NULL without freeing cf, but hot reload still does new_cf = flb_cf_create_from_file(new_cf, file) in src/flb_reload.c:554 and then returns on !new_cf, losing the only pointer to the caller-owned config object. A bad reload config, such as a missing include, will now leak that new_cf and any reconstructed CLI config stored in it on every failed reload attempt; use a temporary return value or update the error path to destroy the original caller-owned object.

Useful? React with 👍 / 👎.