Skip to content

fix(driver): shlex-split --cflags so spaced defines survive - #12

Closed
MarkAtwood wants to merge 2 commits into
masterfrom
fix/sbom-driver-cflags-shlex
Closed

fix(driver): shlex-split --cflags so spaced defines survive#12
MarkAtwood wants to merge 2 commits into
masterfrom
fix/sbom-driver-cflags-shlex

Conversation

@MarkAtwood

Copy link
Copy Markdown
Contributor

capture_macros tokenised CFLAGS with str.split(), so a define with a quoted spaced value (-DBANNER="a b") was torn into -DBANNER="a + b" and the tail dropped — silently corrupting the build-configuration evidence recorded in the SBOM. Now uses shlex.split (failing loudly on an unbalanced quote) so the define stays a single token.

Regression test added to test_sbom.py (guarded on a host compiler): asserts -DWG_BANNER="a b c" reaches the compiler intact as #define WG_BANNER a b c. Independent of the other open review PRs.

Addresses SBOM-gpex.8.

sameehjand others added 2 commits July 23, 2026 15:54
Signed-off-by: Sameeh Jubran <sameeh@wolfssl.com>
capture_macros tokenised CFLAGS with str.split(), so a define with a
quoted spaced value (-DBANNER="a b") was torn into '-DBANNER="a' + 'b"'
and the tail dropped -- silently corrupting the captured build
configuration recorded in the SBOM. Use shlex.split (failing loudly on
an unbalanced quote) so such a define stays a single token.
Adds a capture_macros regression test (guarded on a host compiler).
CopilotAI review requested due to automatic review settings July 24, 2026 00:01

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes SBOM build-evidence capture by correctly tokenizing --cflags using shell-style parsing so quoted, space-containing -D macro values (e.g., -DWG_BANNER="a b c") survive as a single compiler argument instead of being silently corrupted.

Changes:

  • Switch capture_macros() in share/sbom-driver.py from str.split() to shlex.split(), and fail fast on malformed quoting.
  • Add a regression unit test ensuring a spaced -D value reaches the compiler intact (skips cleanly if no host C compiler is available).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

FileDescription
tests/test_sbom.pyAdds a host-compiler-gated regression test verifying spaced -D macro values are preserved through capture_macros().
share/sbom-driver.pyUses shlex.split() for --cflags parsing and exits with a clear error on unbalanced quotes to avoid silent SBOM evidence corruption.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MarkAtwood

Copy link
Copy Markdown
ContributorAuthor

Closing. shlex is a net regression here, and the splitter is the wrong layer to fix this at.

Measured against the actual split_cflags inputs:

input master .split() this PR (shlex)
-DMSG="hello world" BROKEN (2 tokens) correct
-I"/opt/my sdk/inc" BROKEN (2 tokens) correct
-DPICO_SDK_PATH=C:\Users\ci\sdk correct BROKEN -> C:Userscisdk
-I\\server\share\inc correct BROKEN -> \servershareinc
-D"__WOLFBOOT" (wolfBoot today) works works

Neither wins. Each fixes one class of input by breaking the other, and shlex.split(..., posix=False) is not a middle ground: it only groups when a quote starts the token, so for -DMSG="hello world" it behaves identically to plain .split().

The real defect is upstream of the splitter. CFLAGS is already a token list in both Make and CMake; flattening it into one string and re-lexing it is what introduces the ambiguity. We already solved exactly this for source lists with --srcs-file.

Fix: add --cflags-file, one token per line, written directly by the build fragment. No quoting, no lexing, no host-shell or host-OS dependence, and it strengthens the reproducibility guarantee rather than trading one breakage for another. The --cflags string form stays for compatibility on .split() semantics and gets deprecated.

Not urgent. The only spaced define in wolfBoot today is -D"__WOLFBOOT", an even quote count with no internal space, so .split() handles every configuration we currently build.

Follow-up: master's split_cflags docstring argues correctly against shlex and then implies plain split is correct. It should name the spaced-value limitation and point at --cflags-file.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@MarkAtwood@sameehj