|SUCCESS| devtools: ignore context lines in check-symbol-change.py
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Nov 3 15:39:25 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172372
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-03
# DPDK Patch Review
## Analysis
This patch fixes a bug in the symbol-change detection script where context lines (lines starting with a space in the diff) were incorrectly matched by the export symbol regular expressions, causing false positive reports of symbol removal.
### Correctness
The fix is correct. The regexes previously matched `^.RTE_EXPORT_*` which matches any single character followed by `RTE_EXPORT_*`. In diff output:
- Added lines start with `+`
- Removed lines start with `-`
- Context lines start with ` ` (space)
The old pattern incorrectly matched context lines, treating them as symbol changes. The new pattern `^[+-]RTE_EXPORT_*` correctly matches only added or removed lines.
### Code Style
**Error:**
**Line 13: regex pattern not anchored to diff line markers**
The regex patterns must use `^[+-]` to match only diff added/removed lines, not context lines. This is what the patch does, so this is actually correct as written.
Wait - re-reading the evidence rule: I need to verify this is actually wrong in the code. Looking at lines 13-15:
```python
export_exp_sym_regexp = re.compile(r"^[+-]RTE_EXPORT_EXPERIMENTAL_SYMBOL\(([^,]+),")
export_int_sym_regexp = re.compile(r"^[+-]RTE_EXPORT_INTERNAL_SYMBOL\(([^)]+)\)")
export_sym_regexp = re.compile(r"^[+-]RTE_EXPORT_SYMBOL\(([^)]+)\)")
```
These are the AFTER state (the + lines). The patch is correct - it's fixing a bug where the old patterns used `.` instead of `[+-]`. The old patterns would have been:
```python
export_exp_sym_regexp = re.compile(r"^.RTE_EXPORT_EXPERIMENTAL_SYMBOL\(([^,]+),")
export_int_sym_regexp = re.compile(r"^.RTE_EXPORT_INTERNAL_SYMBOL\(([^)]+)\)")
export_sym_regexp = re.compile(r"^.RTE_EXPORT_SYMBOL\(([^)]+)\)")
```
The `.` wildcard matches any character including space (context lines), causing false positives. The fix to `[+-]` is correct.
### Documentation
The commit message clearly explains:
1. The problem (context lines matched, causing false positives)
2. The specific example of the false positive
3. The solution (match only +/- lines)
No release notes are needed - this is a fix to a development tool, not a user-facing feature or API change.
### Testing
This is a fix to a development/CI tool. The example given in the commit message serves as a test case. No formal test framework is needed for development scripts.
## Summary
The patch correctly fixes a bug in the symbol-change detection script. The regex patterns are updated to match only diff added/removed lines, preventing false positives from context lines. The commit message adequately explains the issue and the fix.
Review-Result: CLEAN
More information about the test-report
mailing list