-
Notifications
You must be signed in to change notification settings - Fork 16
Description
Part of duplicate code analysis: #1948
Summary
SecrecyLabel.CanFlowTo and IntegrityLabel.CanFlowTo in internal/difc/labels.go are near-structurally identical: both acquire RLocks on two labels, check for a wildcard tag, then iterate to verify a subset/superset relationship. The checkFlowHelper function already abstracts the CheckFlow method for both types, but CanFlowTo was not unified in the same refactor.
Duplication Details
Pattern: nil-check → dual RLock → wildcard-check → subset/superset loop
- Severity: Low
- Occurrences: 2 functions
- Locations:
internal/difc/labels.golines 173–198 (SecrecyLabel.CanFlowTo, 26 lines)internal/difc/labels.golines 330–356 (IntegrityLabel.CanFlowTo, 27 lines)
Shared structure:
func (l *XLabel) CanFlowTo(target *XLabel) bool {
if l == nil || l.Label == nil { return (semantics-specific-nil-return) }
if target == nil || target.Label == nil { return (semantics-specific-nil-return) }
l.Label.mu.RLock(); defer l.Label.mu.RUnlock()
target.Label.mu.RLock(); defer target.Label.mu.RUnlock()
// Wildcard check (side differs: secrecy checks target; integrity checks l)
if _, ok := (wildcard-side).Label.tags[WildcardTag]; ok { return true }
// Subset/superset loop (iteration side differs)
for tag := range (iter-side).Label.tags {
if _, ok := (check-side).Label.tags[tag]; !ok { return false }
}
return true
}The only semantic differences are:
- Which side (
lvstarget) is the wildcard holder - Which side is iterated for the subset/superset check
- Nil-return value when
l == nil
Impact Analysis
- Maintainability: Adding a new label feature (e.g., negation tags, hierarchical tags) requires updating both
CanFlowToimplementations - Bug Risk: Low — the locking pattern and loop logic are currently identical, but could diverge in future changes
- Code Bloat: ~25 lines of parallel logic
Refactoring Recommendations
-
Extend
checkFlowHelperto serveCanFlowToby calling it from both methods:func (l *SecrecyLabel) CanFlowTo(target *SecrecyLabel) bool { var srcLabel, targetLabel *Label if l != nil { srcLabel = l.Label } if target != nil { targetLabel = target.Label } ok, _ := checkFlowHelper(srcLabel, targetLabel, true, "Secrecy") return ok }
Note:
checkFlowHelperalready implements the full nil/wildcard/subset/superset logic, makingCanFlowToa trivial wrapper. The only difference is the nil-return value whenl == nilforIntegrityLabel(returnstarget == nil || target.Label.IsEmpty()), which the helper would need to preserve.- Estimated effort: 1 hour (plus careful review of nil semantics)
- Benefits: single locking and iteration path; DIFC correctness bugs fixed in one place
-
Alternative: Introduce a
canFlowHelperboolean variant ofcheckFlowHelperthat directly returnsboolwithout collecting violation tags — useful if the tag-collection overhead ofcheckFlowHelperis a concern in hot paths.
Implementation Checklist
- Review duplication findings
- Verify
checkFlowHelpernil semantics matchCanFlowTonil semantics exactly - Refactor both
CanFlowTomethods to delegate tocheckFlowHelper(or a newcanFlowHelper) - Run DIFC label unit tests (
internal/difc/labels_test.go) to confirm correctness
Parent Issue
See parent analysis report: #1948
Related to #1948
Generated by Duplicate Code Detector · ◷
- expires on Mar 22, 2026, 3:06 AM UTC