refactor: use sync.Map in RunExclusive; document discard idioms elsewhere#205
Open
tonyandrewmeyer wants to merge 2 commits into
Open
refactor: use sync.Map in RunExclusive; document discard idioms elsewhere#205tonyandrewmeyer wants to merge 2 commits into
tonyandrewmeyer wants to merge 2 commits into
Conversation
…m elsewhere Triages the 2026-06-01 TIOBE security/AI findings cluster (see canonical-work-queue: roadmap/26.10/repo-setup/tiobe-findings/concierge.csv). helpers.go: replace the cmdMu + map[string]*sync.Mutex pattern in RunExclusive with sync.Map.LoadOrStore. Behaviour is identical (the map only ever grew anyway), but the new shape is statically obvious to Coverity and removes the COV_GO_ATOMICITY hit on the "value captured under lock A, used outside it" idiom. Smaller code as a bonus. Comment-only changes elsewhere — each block documents an existing _ = discard so the next maintainer (and future TIOBE scans, where useful) can see the explicit acknowledgement instead of having to re-derive that pflag's Get*/strings.Builder.Write/etc are unreachable-error APIs in this caller's flow: - cmd/main.go parseLoggingFlags: --verbose / --trace / --dry-run are registered persistent flags on root; pflag.Get* unreachable error. - cmd/prepare.go prepareCmd RunE: same, for --config and --preset. - cmd/restore.go restoreCmd RunE: same, for --dry-run / --verbose / --trace. - internal/config/config.go NewConfig: same, for the four flag reads. - internal/config/config.go envOrFlagBool: docstring extended to cover the sibling envOrFlagString / envOrFlagSlice helpers too — same argument: keys come from getOverrides and are all registered on root. - internal/providers/providers.go buildHostsTomlFromConfig: note that (*strings.Builder).Write never returns a non-nil error, so the five fmt.Fprintf calls here cannot either. All 20 SECURITY findings from the export are reported as TIOBE FPs in parallel — those rationales rely on these comments to keep the "intentional discard" story visible in-tree. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
d561593 to
b0d839c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The primary motivation here is to address the findings from the TIOBE TICS report, according to the cycle's SSDLC requirements.
The only real change is:
helpers.go: replace the cmdMu + map[string]*sync.Mutex pattern in RunExclusive with sync.Map.LoadOrStore. Behaviour is identical (the map only ever grew anyway), but this should be clearer to Coverity and removes the COV_GO_ATOMICITY hit on the "value captured under lock A, used outside it" idiom.The rest is comments, explaining why error checking is not done. We already have a few of these; this should cover the rest. I requested review of all of these in TIOBE, so the comment shouldn't be needed, but I think it will help reviewers and also maintainers of the code understand that these are deliberate choices.