Remove dead C backend code#8
Merged
Merged
Conversation
- Remove mesh_matrix[] buffer (3d visualizer, never used by API) - Remove mesh_values[] double array (parsed values, unused) - Remove mesh_matrix_clear(), mesh_clear(), mesh_matrix_export() functions - Remove parse_mesh_values() — replaced with count_mesh_elements() - Remove probe_count_x, probe_count_y, x_count, y_count globals (set but never read) - Remove get_ssh_status() function (duplicate; api.c has its own inline impl) - Remove MAX_DATA_SLOTS, MIN_SUPPORTED_GRID_SIZE, MESH_MATRIX_ELEMENTS macros
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoved legacy mesh scaffolding and public mesh symbols from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Remove 257-element GRID[] table (integer sqrt via lookup) - Remove BYTE typedef (was only used by GRID[]) - Replace count_mesh_elements()+GRID[] with mesh_grid_size_from_csv(): counts commas, then does a simple integer sqrt loop (n*n==count) - Result is clearer and 250+ lines shorter
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/request.c (1)
192-205: Loop upper bound exceedsMAX_SUPPORTED_GRID_SIZE.The loop iterates up to
n = 12, supporting grids up to 12×12 (144 elements), butMAX_SUPPORTED_GRID_SIZEis defined as 10 on line 171. If a valid 11×11 or 12×12 CSV is parsed,mesh_gridwould be set to 11 or 12, potentially causing downstream issues when code assumes the grid fits within the 10×10 limit (e.g., buffer sizing viaMESH_BUFFER_SIZE).Consider aligning the loop bound with the macro:
Suggested fix
// Integer square root: find n such that n*n == count - for (int n = 2; n <= 12; n++) { + for (int n = 2; n <= MAX_SUPPORTED_GRID_SIZE; n++) { if (n * n == count) return n; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/request.c` around lines 192 - 205, The loop in mesh_grid_size_from_csv currently checks n up to 12, exceeding the project limit MAX_SUPPORTED_GRID_SIZE and allowing mesh_grid values that may overflow buffers like MESH_BUFFER_SIZE; change the loop upper bound to use the macro (for (int n = 2; n <= MAX_SUPPORTED_GRID_SIZE; n++)) so the function only returns grid sizes within the supported limit, and ensure callers that set mesh_grid use that value consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/request.c`:
- Around line 248-252: The code uses strcpy(mesh_config, &b[9]) which can
overflow MESH_BUFFER_SIZE when getline() returns a longer line; change the copy
to a bounded copy that respects MESH_BUFFER_SIZE (e.g., use strncpy/strlcpy or
memcpy with an explicit length calculation) and always null-terminate
mesh_config after copy, and validate/truncate the source (&b[9]) length before
calling mesh_grid_size_from_csv if that function expects a full string; update
the block around mesh_config, mesh_grid_size_from_csv, and any use of b to
enforce the buffer limit and prevent overflow.
---
Nitpick comments:
In `@src/request.c`:
- Around line 192-205: The loop in mesh_grid_size_from_csv currently checks n up
to 12, exceeding the project limit MAX_SUPPORTED_GRID_SIZE and allowing
mesh_grid values that may overflow buffers like MESH_BUFFER_SIZE; change the
loop upper bound to use the macro (for (int n = 2; n <= MAX_SUPPORTED_GRID_SIZE;
n++)) so the function only returns grid sizes within the supported limit, and
ensure callers that set mesh_grid use that value consistently.
- Replace strcpy with strncpy(MESH_BUFFER_SIZE-1) + null-termination - Prevents buffer overflow from long lines in config file
- request.c: remove unused 'i' in update_printer_config_file - request.c: silence unused 'arg' in webcam_capture_thread (pthread signature) - request.c: remove unused 'co' in process_custom_pages - api.c: silence unused 'filename' in handle_api_request (handler signature) - config-parser.c: remove unused 'k_or_v' (set but never read) - response.c: remove unused 'expires' in mkheader - webcam.c: remove unused 'file_size' in v_process_image - webfsd.c: silence unused 'thread_arg' in mainloop (pthread signature)
Average z_offset like 1.4111333333333331 was being written to config. Klipper expects 6 decimal places (e.g. -1.440000). Add round6() helper and apply it to avgZOffset and mesh point precision rounding.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/lib/stores/leveling.ts`:
- Around line 51-53: The current round6 function uses Math.round which
implements asymmetric tie-breaking for negatives and thus doesn't match the C
backend's round() (which rounds half away from zero); update round6 to perform
symmetric half-away-from-zero rounding at the 1e-6 scale by multiplying val by
1e6, taking the absolute value, adding 0.5 and flooring to get the rounded
magnitude, then reapply the original sign and divide by 1e6 so that halfway
values (e.g., -1.500000) round away from zero like C's round(); ensure this
change is applied in the round6 function.
In `@src/request.c`:
- Around line 201-203: The mesh-size detection loop that tests for n*n == count
must be bounded and must validate the stored/truncated grid size consistently:
clamp or reject any computed size > MAX_SUPPORTED_GRID_SIZE (use
MAX_SUPPORTED_GRID_SIZE in the check where you compute/derive size from count),
compute count from the same bounded/truncated buffer used for storage (so the
value used in the for loop and the stored size copy are consistent), and
explicitly fail (return error/false) if the computed size is 0 or >
MAX_SUPPORTED_GRID_SIZE rather than marking success; update the loop/validation
around the code that uses count, n, MAX_SUPPORTED_GRID_SIZE and the stored size
variable so parsing only succeeds when size is non-zero, <=
MAX_SUPPORTED_GRID_SIZE, and n*n == count.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/lib/stores/leveling.tssrc/api.csrc/config-parser.csrc/request.csrc/response.csrc/webcam.csrc/webfsd.c
💤 Files with no reviewable changes (2)
- src/response.c
- src/webcam.c
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/request.c">
<violation number="1" location="src/request.c:248">
P2: After introducing bounded copy with `strncpy`, `mesh_grid_size_from_csv` is still called with the unbounded `&b[9]` rather than the now-bounded `mesh_config`. If the input exceeds `MESH_BUFFER_SIZE`, `mesh_config` will be truncated but `mesh_grid` will be computed from the full untruncated data — creating an inconsistent state where the stored mesh string and the computed grid size don't match. Pass `mesh_config` to the function instead to ensure consistency.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- leveling.ts: fix round6() to use half-away-from-zero (like C's round()) instead of Math.round() which rounds negatives toward +infinity - request.c: use MAX_SUPPORTED_GRID_SIZE in mesh_grid_size_from_csv loop instead of magic constant 12 - request.c: validate mesh line length before copying to prevent accepting truncated meshes; compute grid size from bounded copy for consistency; fail explicitly if grid size is 0
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
1 participant
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.
Audited the C backend against frontend API usage. The frontend is the only consumer of the C backend API. All active endpoints are used — no dead endpoints were found.
However,
src/request.ccontained dead internal code that accumulated over time:Removed
Dead globals
mesh_matrix[]— 3D visualizer buffer, never read by any API responsemesh_values[]— double array for parsed mesh values, unused externallyprobe_count_x,probe_count_y— set from config but never readx_count,y_count— set from config but never readDead functions
mesh_matrix_clear()— only called by deadmesh_matrix_export()mesh_clear()— only called by deadparse_mesh_values()mesh_matrix_export()— formatted mesh matrix for 3D visualizer, never usedparse_mesh_values()— parsed mesh into double array; replaced withcount_mesh_elements()(counts commas to determine grid size)get_ssh_status()— duplicate;api.chas its own inline SSH process scanDead macros
MAX_DATA_SLOTSMIN_SUPPORTED_GRID_SIZEMESH_MATRIX_ELEMENTSNet change
-96 lines (106 deleted, 10 added for
count_mesh_elements())All 30 e2e tests pass.
Assisted-by: 🤖 Claude Sonnet 4.6
Summary by CodeRabbit
Refactor
Bug Fixes
Chores