lib: helper: simplify and extend _BF_APPLY macros

[yaakov-stein]

• Use recursive chaining where each _BF_APPLYn calls _BF_APPLYn-1.
• Extend from a max of 10 arguments to 16 (likely will be necessary for Generate BPF_PROG_TYPE_CGROUP_SOCK_ADDR BPF programs #355).
• Update BF_FLAG to use 1ULL to avoid UB when enum values reach 31+ (bf_matcher already has 31 values).

Note:

• If we need to support more flags at some point, we may want to look in to the Swanson Map Macro. For now, that is probably making things more complicated than they need to be.

Testing:

• CI/Existing tests
• Manual Testing

[github-actions]

Claude: Review of PR #437 — lib: helper: simplify and extend _BF_APPLY macros

This PR is clean and well-motivated. The recursive chaining approach is semantically equivalent to the original manual expansion, reduces code from ~55 lines to ~18, and extends the limit from 10 to 16 arguments for future use. The BF_FLAG change from 1 to 1ULL is a correct proactive fix for UB when enum indices reach 31+.

Verified:

• Recursive macro expansion produces identical output for all arg counts (1–10)
• __BF_NUM_ARGS1 / _BF_NUM_ARGS correctly count up to 16 args
• 16 levels of macro recursion is well within compiler limits
• All existing callers (32 files using BF_FLAG/BF_FLAGS) are compatible with the unsigned long long type change — the project uses -Wall -Wextra but not -Wconversion, and all storage types (uint8_t, uint16_t, uint32_t) handle implicit narrowing correctly for the value ranges in use
• ~BF_FLAG() patterns in opts.c and print.c work correctly despite wider mask
• Max observed BF_FLAGS usage is 6 args, so the 16-arg limit has good margin

One minor suggestion (left as inline comment): consider adding a static_assert to guard against enum growth exceeding 64 values, since 1ULL << n for n ≥ 64 is still UB.

No regressions identified.

[qdeslandes]

LGTM except for a comment to fix.

If we need to support more flags at some point, we may want to look in to the Swanson Map Macro. For now, that is probably making things more complicated than they need to be.

Agreed, let's keep it this way for now and implement it when we reach the limit.

[github-actions]

Claude: review of facebook/bpfilter #437 (d6969d5)

Suggestions

• Commit subject omits BF_FLAG UB fix — commit message — The subject lib: helper: simplify and extend _BF_APPLY macros only describes the macro refactoring. The commit also fixes potential UB by changing BF_FLAG from (1 << (n)) to (1ULL << (n)), adds a static_assert to matcher.h, and removes a redundant assert from set.c. A broader subject like lib: helper: simplify _BF_APPLY macros and fix BF_FLAG UB (55 chars) would better represent the changes.

Nits

• @brief not updated for 1ULL change (dismissed) — src/libbpfilter/include/bpfilter/helper.h:79 — Author prefers keeping the @brief as the human-readable version.
• /// @todo comment style on static_assert (dismissed) — src/libbpfilter/include/bpfilter/matcher.h:116 — Author notes that /// is used for single-line @todo comments throughout the codebase.

Overall this is a clean, well-motivated PR. The recursive macro chaining is correct, the BF_FLAG UB fix is safe for all existing callers, and the static_assert placement is an improvement over the old runtime-path assert.

[qdeslandes]

LGTM, thanks! :D

[qdeslandes]

Actually, LGTM but needs rebase ;)