rcx: refactor to isolate extraction rules parsing logic into its own function#10909
rcx: refactor to isolate extraction rules parsing logic into its own function#10909AcKoucher wants to merge 7 commits into
Conversation
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
1) Move corner-related variables clearing to register method. 2) Tiny refactor for readability. 3) Rename parseRules corner-table parameter to extractor_corner_table. 4) Make parseRules a free factory function in the rcx namespace. 5) Make registerRulesModel take a raw pointer instead of a unique_ptr. Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the extraction rules parsing and corner registration flow by replacing the setCorners method with a standalone parseRules function and a registerRulesModel method, removing the member variable rules_file_path_. Feedback points out a critical stack buffer overflow risk in parseRules where a fixed-size array of size 10 is populated without bounds checking, as well as a potential null pointer dereference when calling fclose on a file handle that failed to open.
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa937409eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
Summary
The multi chip extractor will be the owner of the rules model (the internal representation of the extraction rules).
What's really going on here is: the function
setCornersis being split as it was doing several things at the same time:parseRules.registerRulesModel.This PR also:
dbTechthe source of truth for the extraction rules file.The result of this refactoring is far from ideal. In other to fully address the multiple inconsistencies that exist in the code, we'll need considerably more refactoring rounds. As the current priority is 3D extraction, I'll tackle this in the future.
Type of Change
Impact
None.
Verification
./etc/Build.sh).