|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