clang-tidy: fix warnings of cppcoreguidelines-init-variables#3083
clang-tidy: fix warnings of cppcoreguidelines-init-variables#3083ayushjariyal wants to merge 7 commits intopgRouting:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds explicit NULL/zero initializations to many previously uninitialized local variables (FuncCallContext, TupleDesc, MemoryContext oldcontext, HeapTuple, Datum, Datum*, bool*, loop indices, SPI locals) across numerous SRF and helper C functions; no control-flow or public API changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
@cvvergara, Please review this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/coloring/edgeColoring.c (1)
128-141:⚠️ Potential issue | 🟠 MajorFix uninitialized third array element: reduce num from 3 to 2.
The SQL function
_pgr_edgeColoringdeclares exactly 2 output columns (edge_idandcolor_id), but the C code allocates arrays for 3 elements and only initializes the first 2. The uninitializedvalues[2]is then passed toheap_form_tuple, causing undefined behavior.Fix
- size_t num = 3; + size_t num = 2;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/coloring/edgeColoring.c` around lines 128 - 141, The arrays 'values' and 'nulls' are overallocated to 3 but the SQL function `_pgr_edgeColoring` returns only 2 columns, leaving values[2] uninitialized and passed into heap_form_tuple; change the allocation/count variable 'num' from 3 to 2, allocate values and nulls with num (2) entries, ensure the loop that sets nulls runs for i < num, and only assign values[0] and values[1] before calling heap_form_tuple so no uninitialized element is passed.src/planar/boyerMyrvold.c (1)
78-127:⚠️ Potential issue | 🔴 CriticalType mismatch:
NULLassigned tosize_tvariable.Line 124 initializes
size_t iwithNULL, which is a pointer constant. This is incorrect for an integer type and may cause compiler warnings or undefined behavior.All other files in this PR correctly use
size_t i = 0;.🐛 Proposed fix
- size_t i = NULL; + size_t i = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/planar/boyerMyrvold.c` around lines 78 - 127, In _pgr_boyermyrvold, the local variable declaration "size_t i = NULL;" is a type mismatch (NULL is a pointer constant); change the initializer to an integer zero (e.g., "size_t i = 0;") so the loop variable i has the correct type/value; update the declaration near the loop that sets nulls[i] to false inside the SRF per-call block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/coloring/edgeColoring.c`:
- Around line 128-141: The arrays 'values' and 'nulls' are overallocated to 3
but the SQL function `_pgr_edgeColoring` returns only 2 columns, leaving
values[2] uninitialized and passed into heap_form_tuple; change the
allocation/count variable 'num' from 3 to 2, allocate values and nulls with num
(2) entries, ensure the loop that sets nulls runs for i < num, and only assign
values[0] and values[1] before calling heap_form_tuple so no uninitialized
element is passed.
In `@src/planar/boyerMyrvold.c`:
- Around line 78-127: In _pgr_boyermyrvold, the local variable declaration
"size_t i = NULL;" is a type mismatch (NULL is a pointer constant); change the
initializer to an integer zero (e.g., "size_t i = 0;") so the loop variable i
has the correct type/value; update the declaration near the loop that sets
nulls[i] to false inside the SRF per-call block.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (60)
src/allpairs/floydWarshall.csrc/allpairs/johnson.csrc/astar/astar.csrc/bdAstar/bdAstar.csrc/bdDijkstra/bdDijkstra.csrc/bellman_ford/bellman_ford.csrc/bellman_ford/bellman_ford_neg.csrc/bellman_ford/edwardMoore.csrc/breadthFirstSearch/binaryBreadthFirstSearch.csrc/breadthFirstSearch/breadthFirstSearch.csrc/chinese/chinesePostman.csrc/circuits/hawickCircuits.csrc/coloring/bipartite.csrc/coloring/edgeColoring.csrc/coloring/sequentialVertexColoring.csrc/common/postgres_connection.csrc/components/articulationPoints.csrc/components/biconnectedComponents.csrc/components/bridges.csrc/components/connectedComponents.csrc/components/makeConnected.csrc/components/strongComponents.csrc/contraction/contractGraph.csrc/contraction/contractionHierarchies.csrc/dagShortestPath/dagShortestPath.csrc/dijkstra/dijkstra.csrc/dijkstra/dijkstraVia.csrc/dominator/lengauerTarjanDominatorTree.csrc/driving_distance/driving_distance.csrc/driving_distance/driving_distance_withPoints.csrc/ksp/ksp.csrc/ksp/turnRestrictedPath.csrc/ksp/withPoints_ksp.csrc/lineGraph/lineGraph.csrc/lineGraph/lineGraphFull.csrc/max_flow/edge_disjoint_paths.csrc/max_flow/max_flow.csrc/max_flow/maximum_cardinality_matching.csrc/max_flow/minCostMaxFlow.csrc/metrics/betweennessCentrality.csrc/mincut/stoerWagner.csrc/ordering/cuthillMckeeOrdering.csrc/ordering/kingOrdering.csrc/ordering/sloanOrdering.csrc/ordering/topologicalSort.csrc/pickDeliver/pickDeliver.csrc/pickDeliver/pickDeliverEuclidean.csrc/planar/boyerMyrvold.csrc/spanningTree/kruskal.csrc/spanningTree/prim.csrc/transitiveClosure/transitiveClosure.csrc/traversal/depthFirstSearch.csrc/trsp/trsp.csrc/trsp/trspVia.csrc/trsp/trspVia_withPoints.csrc/trsp/trsp_withPoints.csrc/tsp/TSP.csrc/tsp/euclideanTSP.csrc/withPoints/withPoints.csrc/withPoints/withPointsVia.c
cvvergara
left a comment
cvvergara
left a comment
There was a problem hiding this comment.
I dont see your actions running
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/max_flow/max_flow.c`:
- Around line 39-42: Remove the duplicated macro invocation by keeping a single
PG_FUNCTION_INFO_V1 for the _pgr_maxflow function: locate both occurrences of
PG_FUNCTION_INFO_V1(_pgr_maxflow) and delete one so only one
PG_FUNCTION_INFO_V1(_pgr_maxflow) remains in the file (ensuring the remaining
macro stays immediately before or near the corresponding _pgr_maxflow function
definition).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/max_flow/max_flow.c`:
- Around line 113-116: The loop variable i is declared outside but
re-initialized in the for header; change the declaration to scope i within the
loop by moving `size_t i` into the for statement (for example: `for (size_t i =
0; i < 6; ++i)`), updating the code that sets `nulls[i] = false` accordingly;
this reduces scope and follows cppcoreguidelines-init-variables while leaving
the `nulls` array and its usage unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e2b019a-3ebf-4d9a-9068-e31c94ee3d7d
📒 Files selected for processing (1)
src/max_flow/max_flow.c
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@cvvergara , Could you please review this? I have updated the code according to the suggestions from CodeRabbit, and the GitHub Actions are running successfully on my fork. |
2 participants
Fixes #3036 ,
while running
run-clang-tidy -checks='-*,cppcoreguidelines-init-variables', I notice that there are still warnings in.cfiles.This PR initializes the affected variables in the
.cfiles toresolve the remaining clang-tidy warnings.
Summary by CodeRabbit