(C++/performance) Removing passing c_str() to a function that takes s… #2725
Conversation
…td::string as argument
WalkthroughThe pull request introduces a systematic modification across multiple source files in the pgRouting library. The primary change involves removing the Changes
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
🔭 Outside diff range comments (6)
src/allpairs/johnson_driver.cpp (1)
Line range hint
1-1: Overall improvement in string handling.The systematic removal of unnecessary
.c_str()calls across the codebase is a good improvement that:
- Reduces potential string lifetime issues
- Improves code clarity
- Better leverages C++ string handling
Consider using a static analyzer to find any remaining instances of unnecessary
.c_str()calls.src/max_flow/maximum_cardinality_matching_driver.cpp (1)
Line range hint
4-4: Track TODO comment for v4 release.The TODO comment indicates that the
directedflag parameter is planned to be removed in v4.Would you like me to create an issue to track this TODO for the v4 release planning?
src/dominator/lengauerTarjanDominatorTree_driver.cpp (1)
Line range hint
1-142: Overall string handling improvements look good.The changes to remove unnecessary
.c_str()calls when passing strings topgr_msgare well-implemented throughout the file. This change improves code clarity and potentially performance by avoiding unnecessary string conversions.Consider adding a comment in the header file documenting that
pgr_msgacceptsstd::stringdirectly, to help prevent future inconsistencies.src/dagShortestPath/dagShortestPath_driver.cpp (1)
Line range hint
141-142: Consider updating all remainingc_str()calls consistently.There's a pattern across all files where
ex.c_str()is still being used in the string exception catch block. Consider updating these remaining instances to maintain consistency with the other string handling improvements.Also applies to: 140-141, 151-152, 157-158
src/bellman_ford/bellman_ford_driver.cpp (1)
Line range hint
1-1: Consider updating all remaining.c_str()calls for consistency.While the changes effectively remove most unnecessary string conversions, there's a consistent pattern across all files where the
.c_str()call in thecatch (const std::string &ex)block was missed. Consider updating these remaining instances for complete consistency across the codebase.src/breadthFirstSearch/binaryBreadthFirstSearch_driver.cpp (1)
Line range hint
52-91: Remove commented-out code blockInstead of keeping commented-out code, consider removing it entirely. If this code needs to be referenced later, it should be tracked in version control history.
-#if 0 -template < class G > -std::deque<pgrouting::Path> -pgr_binaryBreadthFirstSearch( - G &graph, - std::vector <II_t_rt> &combinations, - std::vector < int64_t > sources, - std::vector < int64_t > targets) { - std::sort(sources.begin(), sources.end()); - sources.erase( - std::unique(sources.begin(), sources.end()), - sources.end()); - - std::sort(targets.begin(), targets.end()); - targets.erase( - std::unique(targets.begin(), targets.end()), - targets.end()); - - pgrouting::functions::Pgr_binaryBreadthFirstSearch< G > fn_binaryBreadthFirstSearch; - auto paths = combinations.empty() ? - fn_binaryBreadthFirstSearch.binaryBreadthFirstSearch(graph, sources, targets) - : fn_binaryBreadthFirstSearch.binaryBreadthFirstSearch(graph, combinations); - - return paths; -} - -const size_t MAX_UNIQUE_EDGE_COSTS = 2; -const char COST_ERR_MSG[] = "Graph Condition Failed: Graph should have atmost two distinct non-negative edge costs! " - "If there are exactly two distinct edge costs, one of them must equal zero!"; -#endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (63)
include/c_common/e_report.h(1 hunks)src/allpairs/floydWarshall_driver.cpp(1 hunks)src/allpairs/johnson_driver.cpp(1 hunks)src/alpha_shape/alphaShape_driver.cpp(1 hunks)src/astar/astar_driver.cpp(2 hunks)src/bdAstar/bdAstar_driver.cpp(3 hunks)src/bdDijkstra/bdDijkstra_driver.cpp(3 hunks)src/bellman_ford/bellman_ford_driver.cpp(3 hunks)src/bellman_ford/bellman_ford_neg_driver.cpp(3 hunks)src/bellman_ford/edwardMoore_driver.cpp(3 hunks)src/breadthFirstSearch/binaryBreadthFirstSearch_driver.cpp(5 hunks)src/breadthFirstSearch/breadthFirstSearch_driver.cpp(3 hunks)src/chinese/chinesePostman_driver.cpp(3 hunks)src/circuits/hawickCircuits_driver.cpp(3 hunks)src/coloring/bipartite_driver.cpp(3 hunks)src/coloring/edgeColoring_driver.cpp(3 hunks)src/coloring/sequentialVertexColoring_driver.cpp(3 hunks)src/common/e_report.c(1 hunks)src/components/articulationPoints_driver.cpp(2 hunks)src/components/biconnectedComponents_driver.cpp(2 hunks)src/components/bridges_driver.cpp(2 hunks)src/components/connectedComponents_driver.cpp(2 hunks)src/components/makeConnected_driver.cpp(3 hunks)src/components/strongComponents_driver.cpp(2 hunks)src/contraction/contractGraph_driver.cpp(3 hunks)src/dagShortestPath/dagShortestPath_driver.cpp(3 hunks)src/dijkstra/dijkstraVia_driver.cpp(3 hunks)src/dijkstra/dijkstra_driver.cpp(3 hunks)src/dominator/lengauerTarjanDominatorTree_driver.cpp(3 hunks)src/driving_distance/driving_distance_driver.cpp(3 hunks)src/driving_distance/driving_distance_withPoints_driver.cpp(4 hunks)src/ksp/ksp_driver.cpp(3 hunks)src/ksp/turnRestrictedPath_driver.cpp(2 hunks)src/ksp/withPoints_ksp_driver.cpp(4 hunks)src/lineGraph/lineGraphFull_driver.cpp(2 hunks)src/lineGraph/lineGraph_driver.cpp(3 hunks)src/max_flow/edge_disjoint_paths_driver.cpp(1 hunks)src/max_flow/max_flow_driver.cpp(2 hunks)src/max_flow/maximum_cardinality_matching_driver.cpp(2 hunks)src/max_flow/minCostMaxFlow_driver.cpp(2 hunks)src/metrics/betweennessCentrality_driver.cpp(1 hunks)src/mincut/stoerWagner_driver.cpp(2 hunks)src/ordering/cuthillMckeeOrdering_driver.cpp(3 hunks)src/pickDeliver/pickDeliverEuclidean_driver.cpp(2 hunks)src/pickDeliver/pickDeliver_driver.cpp(4 hunks)src/planar/boyerMyrvold_driver.cpp(2 hunks)src/planar/isPlanar_driver.cpp(1 hunks)src/spanningTree/kruskal_driver.cpp(2 hunks)src/spanningTree/mst_common.cpp(2 hunks)src/spanningTree/prim_driver.cpp(2 hunks)src/spanningTree/randomSpanningTree_driver.cpp(2 hunks)src/topologicalSort/topologicalSort_driver.cpp(2 hunks)src/transitiveClosure/transitiveClosure_driver.cpp(2 hunks)src/traversal/depthFirstSearch_driver.cpp(2 hunks)src/trsp/trspVia_driver.cpp(2 hunks)src/trsp/trspVia_withPoints_driver.cpp(4 hunks)src/trsp/trsp_driver.cpp(2 hunks)src/trsp/trsp_withPoints_driver.cpp(4 hunks)src/tsp/TSP_driver.cpp(2 hunks)src/tsp/euclideanTSP_driver.cpp(3 hunks)src/withPoints/get_new_queries.cpp(1 hunks)src/withPoints/withPointsVia_driver.cpp(4 hunks)src/withPoints/withPoints_driver.cpp(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: Ubuntu Boost (86)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: macos (macos-14)
- GitHub Check: macos (macos-latest)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu clang
- GitHub Check: Check for changelog entry
- GitHub Check: Check queries
🔇 Additional comments (56)
include/c_common/e_report.h (1)
44-44: Documentation update looks good.The example code has been correctly updated to reflect the new string handling approach, removing the unnecessary
.c_str()call.src/planar/isPlanar_driver.cpp (1)
84-87: LGTM! String handling improved.The removal of
.c_str()calls improves code clarity while maintaining functionality.src/allpairs/johnson_driver.cpp (1)
87-95: LGTM! String handling improved.The removal of
.c_str()calls improves code clarity while maintaining functionality.src/max_flow/maximum_cardinality_matching_driver.cpp (1)
75-75: LGTM! String handling improvements look good.The changes correctly remove unnecessary
c_str()calls when passing strings topgr_msg, which aligns with the PR objectives and improves code clarity.Also applies to: 91-91, 94-94, 99-100, 103-103, 108-109, 114-115
src/coloring/edgeColoring_driver.cpp (1)
69-69: LGTM! String handling improvements are well-implemented.The changes correctly remove unnecessary
c_str()calls while maintaining proper error handling and resource cleanup patterns.Also applies to: 87-87, 98-99, 104-105, 108-108, 113-114, 119-120
src/metrics/betweennessCentrality_driver.cpp (1)
90-90: LGTM! String handling improvements are consistent.The changes correctly remove unnecessary
c_str()calls while maintaining the integrity of graph processing and error handling logic.Also applies to: 98-98, 103-104, 107-107, 112-113, 118-119
src/components/strongComponents_driver.cpp (1)
76-76: LGTM! String handling improvements are well-executed.The changes correctly remove unnecessary
c_str()calls while maintaining proper memory management and component analysis functionality.Also applies to: 103-103, 106-106, 111-112, 115-115, 120-121, 126-127
src/components/bridges_driver.cpp (1)
74-74: LGTM! Removing unnecessary.c_str()calls improves performance.The changes correctly eliminate redundant string conversions when passing
std::stringobjects topgr_msg.Also applies to: 103-103, 106-106, 111-112, 120-121, 126-127
src/components/connectedComponents_driver.cpp (1)
75-75: LGTM! String handling improvements are consistent.The changes correctly eliminate redundant string conversions when passing
std::stringobjects topgr_msg.Also applies to: 102-102, 105-105, 110-111, 119-120, 125-126
src/components/biconnectedComponents_driver.cpp (1)
76-76: LGTM! String handling improvements are well-implemented.The changes correctly eliminate redundant string conversions when passing
std::stringobjects topgr_msg.Also applies to: 104-104, 107-107, 112-113, 121-122, 127-128
src/components/articulationPoints_driver.cpp (2)
75-75: LGTM! String handling improvements are well-implemented.The changes correctly eliminate redundant string conversions when passing
std::stringobjects topgr_msg.Also applies to: 105-105, 108-108, 113-114, 122-123, 128-129
Line range hint
1-132: Consider updating all string exception handlers systematically.I notice a pattern across all reviewed files where
.c_str()is still being used in the string exception handler while it has been removed everywhere else. Consider updating this pattern systematically across all files for consistency.Let's verify if this pattern exists in other files:
src/topologicalSort/topologicalSort_driver.cpp (1)
88-88: LGTM! String handling improvements look good.The changes correctly remove unnecessary
.c_str()calls when passing strings topgr_msg. Note that the.c_str()call on line 129 is correctly retained as it's necessary for the string exception handling.Also applies to: 118-118, 121-121, 126-127, 130-130, 135-136, 141-142
src/planar/boyerMyrvold_driver.cpp (1)
100-100: LGTM! String handling improvements look good.The changes correctly remove unnecessary
.c_str()calls when passing strings topgr_msg. Note that the.c_str()call on line 124 is correctly retained as it's necessary for the string exception handling.Also applies to: 113-113, 116-116, 121-122, 125-125, 130-131, 136-137
src/chinese/chinesePostman_driver.cpp (1)
75-75: LGTM! String handling improvements look good.The changes correctly remove unnecessary
.c_str()calls when passing strings topgr_msg. Note that the.c_str()call on line 128 is correctly retained as it's necessary for the string exception handling.Also applies to: 104-104, 117-117, 120-120, 125-126, 129-129, 134-135, 140-141
src/components/makeConnected_driver.cpp (1)
77-77: LGTM! String handling improvements look good.The changes correctly remove unnecessary
.c_str()calls when passing strings topgr_msg. Note that the.c_str()call on line 121 is correctly retained as it's necessary for the string exception handling.This pattern is consistent across all reviewed files, where:
- Unnecessary
.c_str()calls are removed when passing string streams topgr_msg.c_str()is retained only where necessary (string exception handling)Also applies to: 96-96, 110-110, 113-113, 118-119, 122-122, 127-128, 133-134
src/lineGraph/lineGraph_driver.cpp (1)
134-135: Verify if.c_str()is needed for string exception.While most
.c_str()calls have been removed, there's still one instance where it's used with the string exception. Consider if this can also be removed for consistency.✅ Verification successful
Keep
.c_str()for string exception handlingThe use of
.c_str()with string exceptions is a consistent pattern across the entire codebase, specifically used when passing caught string exceptions topgr_msg. This appears to be an intentional design choice and should be maintained for consistency.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for patterns of exception handling with string literals rg "catch \(const std::string &.*\).*pgr_msg\(.*\)" -A 2Length of output: 58
Script:
#!/bin/bash # Search for string exception handling patterns echo "=== String Exception Patterns ===" rg "catch.*string" -A 2 echo -e "\n=== pgr_msg Usage Patterns ===" rg "pgr_msg\(" -A 1Length of output: 70168
src/spanningTree/randomSpanningTree_driver.cpp (1)
139-140: Same observation about.c_str()usage in string exception handling.src/spanningTree/kruskal_driver.cpp (1)
135-136: Same observation about.c_str()usage in string exception handling.src/breadthFirstSearch/breadthFirstSearch_driver.cpp (2)
143-144: Same observation about.c_str()usage in string exception handling.
Line range hint
85-156: Overall changes look good and consistent.The modifications to remove
.c_str()when passing strings topgr_msgare consistent across all files and align with the PR objective. The error handling structure and functionality remain unchanged, maintaining the robustness of the error reporting system.src/bdAstar/bdAstar_driver.cpp (1)
96-96: LGTM! String handling improvements.The changes correctly remove unnecessary
c_str()calls when passing strings topgr_msg, improving code clarity and performance.Also applies to: 121-121, 130-130, 133-133, 138-139, 147-148, 153-154
src/driving_distance/driving_distance_driver.cpp (1)
84-84: LGTM! String handling improvements.The changes correctly remove unnecessary
c_str()calls when passing strings topgr_msg, improving code clarity and performance.Also applies to: 106-106, 129-129, 132-132, 137-138, 146-147, 152-153
src/astar/astar_driver.cpp (1)
131-131: LGTM! String handling improvements.The changes correctly remove unnecessary
c_str()calls when passing strings topgr_msg, improving code clarity and performance.Also applies to: 140-140, 143-143, 148-149, 157-158, 163-164
src/dagShortestPath/dagShortestPath_driver.cpp (1)
115-115: LGTM! String handling improvements.The changes correctly remove unnecessary
c_str()calls when passing strings topgr_msg, improving code clarity and performance.Also applies to: 137-137, 146-146, 149-149, 154-155, 163-164, 169-170
src/ksp/ksp_driver.cpp (1)
105-105: LGTM! Performance improvement by removing unnecessary string conversions.The changes correctly eliminate redundant
.c_str()calls when passing strings topgr_msg, which can improve performance by avoiding unnecessary string conversions.Also applies to: 129-129, 144-144, 147-147, 152-153, 156-156, 161-162, 167-168
src/max_flow/minCostMaxFlow_driver.cpp (1)
108-108: LGTM! Consistent performance improvement pattern.The changes align with the codebase-wide effort to eliminate unnecessary string conversions.
Also applies to: 145-145, 148-148, 153-154, 157-157, 162-163, 168-169
src/transitiveClosure/transitiveClosure_driver.cpp (1)
124-124: LGTM! Performance improvements through string handling optimization.The changes consistently remove unnecessary string conversions across the file.
Also applies to: 139-139, 142-142, 147-148, 151-151, 156-157, 162-163
src/bellman_ford/bellman_ford_driver.cpp (1)
116-116: LGTM! Consistent performance optimization pattern.The changes align with the project-wide effort to optimize string handling.
Also applies to: 139-139, 148-148, 151-151, 156-157, 160-160, 165-166, 171-172
src/traversal/depthFirstSearch_driver.cpp (1)
152-152: LGTM! String handling improvements.The removal of
.c_str()calls when passing strings topgr_msgis correct and aligns with the performance improvement goals.Also applies to: 165-165, 168-168, 173-174, 182-183, 188-189
src/max_flow/edge_disjoint_paths_driver.cpp (1)
189-189: LGTM! String handling improvements.The removal of
.c_str()calls when passing strings topgr_msgis correct and aligns with the performance improvement goals.Also applies to: 192-192, 197-198, 203-204, 212-213
src/ksp/withPoints_ksp_driver.cpp (1)
120-120: LGTM! String handling improvements.The removal of
.c_str()calls when passing strings topgr_msgis correct and aligns with the performance improvement goals.Also applies to: 133-134, 166-166, 183-183, 190-190, 193-193, 198-199, 207-208, 213-214
src/driving_distance/driving_distance_withPoints_driver.cpp (2)
103-103: LGTM! String handling improvements.The removal of
.c_str()calls when passing strings topgr_msgis correct and aligns with the performance improvement goals.Also applies to: 117-118, 151-151, 181-181, 184-184, 189-190, 198-199, 204-205
Line range hint
1-1: Verify complete removal of .c_str() calls.Let's verify that we haven't missed any instances of
.c_str()being passed topgr_msg.src/ksp/turnRestrictedPath_driver.cpp (1)
127-127: Verify pgr_msg function signature before removing c_str()The changes remove
.c_str()calls when passing strings topgr_msg. While this improves code clarity and potentially performance, we should verify thatpgr_msgacceptsstd::stringparameters directly.Run this script to check the
pgr_msgfunction declaration:Also applies to: 210-210, 213-213, 219-220, 225-226, 229-229, 234-235
✅ Verification successful
Verified: pgr_msg accepts std::string parameters directly
The codebase shows consistent usage of
pgr_msgwith bothstd::stringand C-style strings. Removing.c_str()is safe and improves code clarity.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify pgr_msg function accepts std::string # Search for pgr_msg declaration in header files rg -t hpp -t h "pgr_msg" -A 2Length of output: 57
Script:
#!/bin/bash # Find header and source files that might contain pgr_msg fd -e h -e hpp -e hxx -e cpp . | xargs rg "pgr_msg" -B 2 -A 2 # Alternative: direct pattern search in case pgr_msg is defined differently rg "pgr_msg.*\(" -B 2 -A 2Length of output: 67370
src/pickDeliver/pickDeliverEuclidean_driver.cpp (1)
144-145: Changes are consistent with the codebase-wide updateThe removal of
.c_str()calls is consistent with the changes in other files, making the code more idiomatic C++.Also applies to: 184-184, 187-187, 192-193, 198-199, 202-202, 209-210, 215-216
src/withPoints/withPointsVia_driver.cpp (1)
145-145: Changes align with modern C++ practicesThe removal of
.c_str()calls:
- Makes the code more idiomatic C++
- Potentially reduces unnecessary string copies
- Improves code readability
Also applies to: 162-163, 206-206, 216-216, 219-219, 224-225, 228-228, 233-234, 239-240
src/withPoints/withPoints_driver.cpp (1)
141-141: LGTM! String handling improvementsThe changes consistently remove unnecessary
.c_str()calls when passing strings topgr_msg, which:
- Improves type safety
- Reduces unnecessary string conversions
- Better manages string lifetimes
Also applies to: 157-158, 208-208, 217-217, 220-220, 225-226, 229-229, 234-235, 240-241
src/trsp/trsp_driver.cpp (1)
175-175: LGTM! Consistent string handling improvementsThe changes align with the project-wide effort to improve string handling by removing unnecessary
.c_str()calls.Also applies to: 227-227, 230-230, 235-236, 241-242, 245-245, 250-251
src/breadthFirstSearch/binaryBreadthFirstSearch_driver.cpp (1)
176-176: LGTM! String handling improvementsThe changes consistently improve string handling by removing unnecessary
.c_str()calls.Also applies to: 187-187, 198-198, 213-213, 223-223, 226-226, 231-232, 235-235, 240-241, 246-247
src/pickDeliver/pickDeliver_driver.cpp (1)
124-124: LGTM! String handling improvementsThe changes consistently improve string handling across all error handling paths by removing unnecessary
.c_str()calls.Also applies to: 135-135, 143-143, 159-160, 203-203, 206-206, 211-212, 217-218, 221-221, 228-229, 236-237, 242-243
src/contraction/contractGraph_driver.cpp (1)
209-209: LGTM! String handling improvementsThe changes consistently remove unnecessary
.c_str()calls when passing strings topgr_msg, which is a good optimization. The error handling patterns remain robust and consistent throughout the file.Also applies to: 221-221, 254-254, 257-257, 262-263, 266-266, 271-272, 277-278
src/trsp/trsp_withPoints_driver.cpp (1)
136-136: LGTM! Consistent string handling improvementsThe changes maintain the robust error handling while optimizing string operations by removing unnecessary
.c_str()calls. The core routing logic remains unaffected.Also applies to: 158-159, 197-197, 260-260, 263-263, 268-269, 274-275, 278-278, 283-284
src/trsp/trspVia_withPoints_driver.cpp (1)
183-183: LGTM! String handling optimizationsThe changes consistently improve string handling while maintaining the robust error handling patterns. The core routing logic with via points remains unaffected.
Also applies to: 202-203, 243-243, 292-292, 295-295, 300-301, 304-304, 309-310, 315-316
src/tsp/TSP_driver.cpp (1)
Line range hint
1-1: Consider using a more systematic approach to remove all.c_str()calls.While the changes effectively remove most
.c_str()calls when passing strings topgr_msg, there are several instances that were missed, particularly in exception handling blocks. Consider using a systematic approach (like grep or ast-grep) to identify and remove all unnecessary.c_str()calls consistently across the codebase.Run this script to find any remaining
.c_str()calls passed topgr_msg:src/ordering/cuthillMckeeOrdering_driver.cpp (1)
101-101: LGTM! String handling improvements look good.The changes correctly remove unnecessary
.c_str()calls when passing strings topgr_msg, which is a good optimization as it eliminates redundant string conversions.Let's verify the
pgr_msgfunction signature to confirm it acceptsstd::string:Also applies to: 117-117, 129-129, 132-132, 137-138, 141-141, 146-147, 152-153
✅ Verification successful
Verified:
pgr_msgcorrectly acceptsstd::stringparameterFound in
include/cpp_common/alloc.hpp:
- Function signature:
pgr_msg(const std::string &msg)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for pgr_msg function declaration rg -A 2 "pgr_msg.*\(" include/Length of output: 311
src/spanningTree/prim_driver.cpp (1)
110-110: LGTM! String handling improvements are consistent.The changes correctly optimize string handling by removing unnecessary
.c_str()calls, maintaining consistency with the codebase-wide improvement initiative.Also applies to: 126-126, 129-129, 134-135, 138-138, 143-144, 149-150
src/lineGraph/lineGraphFull_driver.cpp (1)
97-97: LGTM! String handling improvements are well-implemented.The changes correctly optimize string handling by removing unnecessary
.c_str()calls. The debug section at lines 131-132 is properly maintained while following the same optimization pattern.Also applies to: 131-132, 137-137, 140-140, 145-146, 149-149, 154-155, 160-161
src/bellman_ford/edwardMoore_driver.cpp (1)
116-116: LGTM! String handling improvements are properly implemented.The changes correctly optimize string handling by removing unnecessary
.c_str()calls, maintaining consistency across all message handling sections.Also applies to: 139-139, 148-148, 151-151, 156-157, 160-160, 165-166, 171-172
src/alpha_shape/alphaShape_driver.cpp (1)
183-183: LGTM! String handling improvements include geometry text handling.The changes correctly optimize string handling by removing unnecessary
.c_str()calls. Notable is the consistent handling of both regular messages and geometry text strings (WKT format) through the same optimization pattern.Also applies to: 191-191, 199-199, 202-202, 207-208, 211-211, 216-217, 222-223
src/trsp/trspVia_driver.cpp (6)
210-210: LGTM! Correctly removed.c_str()call.The change properly passes the string directly to
pgr_msgwithout unnecessary conversion.
262-265: LGTM! Properly simplified string handling in message logging.The changes correctly remove unnecessary string conversions while maintaining the original logic.
270-271: LGTM! Error handling properly simplified.The changes correctly streamline string handling in the AssertFailedException catch block.
274-274: LGTM! Proper string handling in exception logging.The change correctly maintains necessary string conversions while removing unnecessary ones.
279-280: LGTM! Exception handling properly simplified.The changes correctly streamline string handling in the std::exception catch block.
285-286: LGTM! Catch-all error handling properly simplified.The changes correctly streamline string handling in the catch-all block.
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (10)
src/pickDeliver/pickDeliverEuclidean_driver.cpp (1)
Line range hint
64-212: Excellent implementation of string handling improvements.The changes in this file are well-implemented and align perfectly with the PR objectives:
- Successfully eliminated unnecessary
.c_str()calls- Maintained consistent error handling and logging behavior
- Improved performance by reducing string conversions
- Applied changes systematically across all message handling code paths
The implementation is thorough and maintains high code quality standards.
Consider documenting the performance benefits of using
to_pg_msgover.c_str()in the codebase's best practices guide to ensure consistent usage across the project.src/max_flow/maximum_cardinality_matching_driver.cpp (1)
Line range hint
4-4: Track TODO comment for v4 release.The TODO comment indicates that the directed flag should be removed in v4. This should be tracked to ensure it's not overlooked.
Would you like me to create an issue to track the removal of the directed flag in v4?
src/components/bridges_driver.cpp (1)
Line range hint
85-89: Fix incorrect error message.The error message "No paths found between start_vid and end_vid vertices" is incorrect for this context as this is a bridges detection algorithm, not a path finding algorithm.
- notice << "No paths found between start_vid and end_vid vertices"; + notice << "No bridges found in the graph";src/components/connectedComponents_driver.cpp (1)
Line range hint
86-90: Fix incorrect error message.The error message "No paths found between start_vid and end_vid vertices" is incorrect for this context as this is a connected components algorithm, not a path finding algorithm.
- notice << "No paths found between start_vid and end_vid vertices"; + notice << "No connected components found in the graph";src/components/strongComponents_driver.cpp (1)
Line range hint
89-91: Assign the constructed notice message.The notice message is constructed but never assigned. Consider assigning it:
notice << "No components found"; + *notice_msg = to_pg_msg(notice); return;src/components/biconnectedComponents_driver.cpp (1)
Line range hint
91-93: Assign the constructed notice message.The notice message is constructed but never assigned. Consider assigning it:
notice << "No paths found between start_vid and end_vid vertices"; + *notice_msg = to_pg_msg(notice); return;src/components/articulationPoints_driver.cpp (2)
Line range hint
89-90: Assign the constructed notice message.The notice message is constructed but never assigned. Consider assigning it:
notice << "No paths found between start_vid and end_vid vertices"; + *notice_msg = to_pg_msg(notice); return;
Line range hint
50-56: Well-structured transition frompgr_msgtoto_pg_msg.The consistent replacement of
pgr_msgwithto_pg_msgacross all files improves performance by eliminating unnecessary string conversions. This systematic change enhances code maintainability and efficiency.Also applies to: 55-56, 51-51, 56-56, 55-55
src/spanningTree/randomSpanningTree_driver.cpp (1)
Line range hint
79-79: Fix undefined variable in assertion.The assertion references
total_edgeswhich is not defined in the function scope.Remove or correct the assertion:
- pgassert(total_edges != 0);src/bellman_ford/bellman_ford_driver.cpp (1)
Line range hint
67-67: Excellent systematic improvement in string handling across the codebase.The consistent replacement of
pgr_msgwithto_pg_msgacross all files demonstrates a well-executed systematic improvement. This change:
- Eliminates unnecessary string conversions
- Improves performance by reducing memory operations
- Maintains consistent error handling across the codebase
- Addresses the static analysis issues identified by cppcheck
Also applies to: 82-82, 62-62, 105-105, 82-82
♻️ Duplicate comments (3)
src/alpha_shape/alphaShape_driver.cpp (1)
76-76: 🧹 Nitpick (assertive)Consider additional string optimization opportunities.
While removing
.c_str()is good, there are additional opportunities to optimize string handling:
- Consider using string_view for read-only string parameters
- Move semantics could be used when concatenating strings in log messages
Also applies to: 197-198, 203-204, 206-207, 212-213, 218-219
src/chinese/chinesePostman_driver.cpp (1)
104-104: 🧹 Nitpick (assertive)Consider checking if notice stream is empty before assigning message.
The message assignment could be guarded by checking if the notice stream is empty to avoid unnecessary allocations.
- *log_msg = to_pg_msg(notice); + *log_msg = notice.str().empty() ? nullptr : to_pg_msg(notice);src/components/makeConnected_driver.cpp (1)
108-109: 🧹 Nitpick (assertive)Consider checking if streams are empty before assigning messages.
Both log and notice message assignments could be guarded by empty checks.
- *log_msg = to_pg_msg(log); - *notice_msg = to_pg_msg(notice); + *log_msg = log.str().empty() ? nullptr : to_pg_msg(log); + *notice_msg = notice.str().empty() ? nullptr : to_pg_msg(notice);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (67)
docqueries/ordering/topologicalSort.result(0 hunks)include/c_common/e_report.h(1 hunks)include/cpp_common/alloc.hpp(2 hunks)src/allpairs/floydWarshall_driver.cpp(2 hunks)src/allpairs/johnson_driver.cpp(2 hunks)src/alpha_shape/alphaShape_driver.cpp(2 hunks)src/astar/astar_driver.cpp(4 hunks)src/bdAstar/bdAstar_driver.cpp(3 hunks)src/bdDijkstra/bdDijkstra_driver.cpp(4 hunks)src/bellman_ford/bellman_ford_driver.cpp(3 hunks)src/bellman_ford/bellman_ford_neg_driver.cpp(4 hunks)src/bellman_ford/edwardMoore_driver.cpp(3 hunks)src/breadthFirstSearch/binaryBreadthFirstSearch_driver.cpp(6 hunks)src/breadthFirstSearch/breadthFirstSearch_driver.cpp(4 hunks)src/chinese/chinesePostman_driver.cpp(4 hunks)src/circuits/hawickCircuits_driver.cpp(4 hunks)src/coloring/bipartite_driver.cpp(4 hunks)src/coloring/edgeColoring_driver.cpp(4 hunks)src/coloring/sequentialVertexColoring_driver.cpp(4 hunks)src/common/alloc.cpp(1 hunks)src/common/e_report.c(1 hunks)src/components/articulationPoints_driver.cpp(3 hunks)src/components/biconnectedComponents_driver.cpp(3 hunks)src/components/bridges_driver.cpp(3 hunks)src/components/connectedComponents_driver.cpp(3 hunks)src/components/makeConnected_driver.cpp(4 hunks)src/components/strongComponents_driver.cpp(3 hunks)src/contraction/contractGraph_driver.cpp(4 hunks)src/cpp_common/trsp_pgget.cpp(2 hunks)src/dagShortestPath/dagShortestPath_driver.cpp(4 hunks)src/dijkstra/dijkstraVia_driver.cpp(4 hunks)src/dijkstra/dijkstra_driver.cpp(4 hunks)src/dominator/lengauerTarjanDominatorTree_driver.cpp(4 hunks)src/driving_distance/driving_distance_driver.cpp(4 hunks)src/driving_distance/driving_distance_withPoints_driver.cpp(5 hunks)src/ksp/ksp_driver.cpp(4 hunks)src/ksp/turnRestrictedPath_driver.cpp(3 hunks)src/ksp/withPoints_ksp_driver.cpp(6 hunks)src/lineGraph/lineGraphFull_driver.cpp(3 hunks)src/lineGraph/lineGraph_driver.cpp(4 hunks)src/max_flow/edge_disjoint_paths_driver.cpp(4 hunks)src/max_flow/max_flow_driver.cpp(6 hunks)src/max_flow/maximum_cardinality_matching_driver.cpp(3 hunks)src/max_flow/minCostMaxFlow_driver.cpp(4 hunks)src/metrics/betweennessCentrality_driver.cpp(2 hunks)src/mincut/stoerWagner_driver.cpp(3 hunks)src/ordering/cuthillMckeeOrdering_driver.cpp(4 hunks)src/pickDeliver/pickDeliverEuclidean_driver.cpp(4 hunks)src/pickDeliver/pickDeliver_driver.cpp(6 hunks)src/planar/boyerMyrvold_driver.cpp(3 hunks)src/planar/isPlanar_driver.cpp(2 hunks)src/spanningTree/kruskal_driver.cpp(4 hunks)src/spanningTree/mst_common.cpp(3 hunks)src/spanningTree/prim_driver.cpp(4 hunks)src/spanningTree/randomSpanningTree_driver.cpp(3 hunks)src/topologicalSort/topologicalSort_driver.cpp(3 hunks)src/transitiveClosure/transitiveClosure_driver.cpp(3 hunks)src/traversal/depthFirstSearch_driver.cpp(4 hunks)src/trsp/trspVia_driver.cpp(4 hunks)src/trsp/trspVia_withPoints_driver.cpp(5 hunks)src/trsp/trsp_driver.cpp(5 hunks)src/trsp/trsp_withPoints_driver.cpp(6 hunks)src/tsp/TSP_driver.cpp(4 hunks)src/tsp/euclideanTSP_driver.cpp(4 hunks)src/withPoints/get_new_queries.cpp(1 hunks)src/withPoints/withPointsVia_driver.cpp(4 hunks)src/withPoints/withPoints_driver.cpp(5 hunks)
💤 Files with no reviewable changes (1)
- docqueries/ordering/topologicalSort.result
🧰 Additional context used
🪛 cppcheck (2.10-2)
src/spanningTree/mst_common.cpp
[style] 58-58: The function 'get_name' is never used.
(unusedFunction)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: macos (macos-14)
- GitHub Check: macos (macos-latest)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (65)
src/pickDeliver/pickDeliverEuclidean_driver.cpp (6)
64-64: LGTM! Good practice with using declaration.The addition of the using declaration for
to_pg_msgis well-placed with other declarations and follows good coding practices.
79-80: LGTM! Proper string handling improvement.The conversion to
to_pg_msgeliminates unnecessary.c_str()calls while maintaining the same error handling logic.
87-88: Consistent with previous changes.
144-145: LGTM! Proper error message handling.The conversion to
to_pg_msgimproves string handling while maintaining the error handling flow.
182-183: LGTM! Consistent message handling.The conversion to
to_pg_msgmaintains proper message handling while improving string handling efficiency.
188-189: LGTM! Comprehensive exception handling improvement.All exception handling blocks have been consistently updated to use
to_pg_msg, improving string handling efficiency while maintaining proper error reporting.Also applies to: 194-195, 197-198, 205-206, 211-212
src/common/alloc.cpp (2)
34-40: LGTM! Good improvements to the string handling function.The changes improve robustness by:
- Adding empty string validation
- Using modern
nullptrinstead ofNULL- Maintaining efficient memory handling
44-47: LGTM! Well-designed overload for ostringstream.The new overload elegantly delegates to the string version, promoting code reuse and consistency.
include/cpp_common/alloc.hpp (2)
36-36: LGTM! Required header for ostringstream support.The addition of
<sstream>header is necessary for the newstd::ostringstreamparameter type.
90-91: LGTM! Clean function declarations.The declarations properly match the implementations and use modern C++ reference parameters.
src/planar/isPlanar_driver.cpp (3)
53-53: LGTM! Updated namespace usage.Correctly updated to use the new
to_pg_msgfunction.
82-83: LGTM! Clean message handling.Direct usage of
to_pg_msgwith string streams improves code clarity.
87-99: LGTM! Consistent error handling.Error handling is thorough and consistent across all catch blocks, properly using
to_pg_msg.src/cpp_common/trsp_pgget.cpp (2)
88-88: LGTM! Updated namespace usage.Correctly updated to use the new
to_pg_msgfunction.
103-107: LGTM! Consistent error handling.Error handling properly uses
to_pg_msgfor both specific and unknown exceptions.Since this file is marked for removal in V4, let's verify if there are any remaining dependencies:
✅ Verification successful
Verified: File and its dependencies are properly marked for removal in V4
All dependencies of
trsp_pgget.cppare properly marked as deprecated and scheduled for removal in V4:
- Used only by
trsp_deprecated.c- Header file explicitly states "TODO V4 Delete this file"
- Listed in CMakeLists.txt as part of old TRSP code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for imports of trsp_pgget rg -l "trsp_pgget"Length of output: 135
Script:
#!/bin/bash # Check how trsp_pgget is used in these files echo "=== trsp_deprecated.c ===" rg -C 3 "trsp_pgget" src/trsp/trsp_deprecated.c echo -e "\n=== CMakeLists.txt ===" rg -C 3 "trsp_pgget" src/cpp_common/CMakeLists.txt echo -e "\n=== Header file ===" cat include/c_common/trsp_pgget.hLength of output: 3161
src/max_flow/maximum_cardinality_matching_driver.cpp (1)
61-61: LGTM! Message handling changes look good.The transition from
pgr_msgtoto_pg_msgis implemented consistently throughout the file, improving string handling efficiency.Also applies to: 74-75, 89-90, 95-96, 98-99, 104-105, 110-111
src/allpairs/floydWarshall_driver.cpp (1)
52-52: LGTM! Message handling changes look good.The transition from
pgr_msgtoto_pg_msgis implemented consistently throughout the file, improving string handling efficiency.Also applies to: 88-88, 94-94, 99-100, 102-103, 108-109, 114-115
src/components/bridges_driver.cpp (1)
54-54: LGTM! Message handling changes look good.The transition from
pgr_msgtoto_pg_msgis implemented consistently throughout the file, improving string handling efficiency.Also applies to: 73-74, 101-102, 107-108, 110-111, 116-117, 122-123
src/components/connectedComponents_driver.cpp (1)
55-55: LGTM! Message handling changes look good.The transition from
pgr_msgtoto_pg_msgis implemented consistently throughout the file, improving string handling efficiency.Also applies to: 74-75, 100-101, 106-107, 109-110, 115-116, 121-122
src/mincut/stoerWagner_driver.cpp (1)
64-64: LGTM! Consistent message handling improvements.The changes correctly replace
pgr_msgwithto_pg_msg, eliminating unnecessary string conversions while maintaining proper error handling. This aligns with the PR's performance improvement objectives.Also applies to: 82-83, 112-113, 118-119, 121-122, 127-128, 133-134
src/topologicalSort/topologicalSort_driver.cpp (1)
69-69: LGTM! Consistent message handling improvements.The changes correctly replace
pgr_msgwithto_pg_msg, eliminating unnecessary string conversions while maintaining proper error handling.Also applies to: 87-88, 116-117, 122-123, 125-126, 131-132, 137-138
src/coloring/sequentialVertexColoring_driver.cpp (1)
68-68: LGTM! Consistent message handling improvements.The changes correctly replace
pgr_msgwithto_pg_msg, eliminating unnecessary string conversions while maintaining proper error handling.Also applies to: 86-87, 106-106, 117-118, 123-124, 126-127, 132-133, 138-139
src/circuits/hawickCircuits_driver.cpp (1)
73-73: LGTM! Consistent message handling improvements.The changes correctly replace
pgr_msgwithto_pg_msg, eliminating unnecessary string conversions while maintaining proper error handling. This also addresses the previous review comment about inconsistent string handling in catch blocks.Also applies to: 91-92, 110-110, 121-122, 127-128, 130-131, 136-137, 142-143
src/tsp/TSP_driver.cpp (1)
120-121: 🛠️ Refactor suggestionRemove unnecessary
.c_str()calls for consistency.For consistency with other changes in the codebase, remove the
.c_str()calls when passing exception strings toto_pg_msg.- *err_msg = to_pg_msg(ex.first.c_str()); - *log_msg = to_pg_msg(ex.second.c_str()); + *err_msg = to_pg_msg(ex.first); + *log_msg = to_pg_msg(ex.second);Likely invalid or redundant comment.
include/c_common/e_report.h (1)
44-44: Documentation updated correctly!The example code has been properly updated to reflect the new API usage, removing the unnecessary
.c_str()call.src/withPoints/get_new_queries.cpp (1)
50-50: LGTM! String handling improvements.The changes correctly remove unnecessary
.c_str()calls and use the newto_pg_msgfunction consistently.Also applies to: 58-58, 66-66
src/spanningTree/mst_common.cpp (2)
38-38: LGTM! String handling improvements.The changes correctly remove unnecessary
.c_str()calls and use the newto_pg_msgfunction consistently.Also applies to: 48-48, 51-51, 59-59, 71-71, 79-79
75-75: 🛠️ Refactor suggestionRemove unnecessary
.c_str()call.For consistency with other changes, remove the
.c_str()call when passing the string toto_pg_msg.- char * full_name = to_pg_msg(name.c_str()); + char * full_name = to_pg_msg(name);Likely invalid or redundant comment.
src/common/e_report.c (1)
47-47: Documentation updated correctly!The example code has been properly updated to reflect the new API usage, removing the unnecessary
.c_str()call.src/allpairs/johnson_driver.cpp (1)
51-51: LGTM! Error handling improvements.The changes correctly update all error handling and logging calls to use
to_pg_msg, removing unnecessary.c_str()calls consistently throughout the file.Also applies to: 87-87, 93-93, 98-99, 101-102, 107-108, 113-114
src/metrics/betweennessCentrality_driver.cpp (1)
90-90: LGTM! Clean and consistent implementation.The changes from
pgr_msgtoto_pg_msgare implemented consistently throughout the error handling paths.Also applies to: 96-96, 101-102, 104-105, 110-111, 116-117
src/coloring/bipartite_driver.cpp (1)
Line range hint
67-73: Good use of NOLINT directives.The NOLINT directives are appropriately used to suppress false positives from clang-analyzer for pointer assertions.
src/dominator/lengauerTarjanDominatorTree_driver.cpp (2)
59-59: LGTM! Consistent message handling changes.The changes to use
to_pg_msgare consistent with the project-wide update pattern and improve string handling safety.Also applies to: 77-78, 100-101, 111-112, 117-118, 120-121, 126-127, 132-133
56-56: Verify complete removal of c_str() calls.Let's verify that all unnecessary
c_str()calls have been removed from message handling code.✅ Verification successful
All c_str() calls successfully removed ✅
The codebase is now free of unnecessary
c_str()calls in message handling code, and no legacypgr_msgusage remains.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining c_str() calls in message handling code rg --type cpp 'c_str\(\).*pgr_msg|pgr_msg.*c_str\(\)' || echo "No remaining c_str() calls found in message handling" # Search for any remaining pgr_msg usage that should be replaced with to_pg_msg rg --type cpp 'pgr_msg' || echo "No remaining pgr_msg usage found"Length of output: 269
src/tsp/euclideanTSP_driver.cpp (2)
58-58: LGTM! String handling improvements look good.The changes correctly replace
pgr_msgwithto_pg_msgand remove unnecessary string conversions, which aligns with the PR objectives to improve performance.Also applies to: 71-72, 81-81, 87-87, 107-108, 113-114, 120-121, 126-127, 132-133
117-118: 🛠️ Refactor suggestionRemove unnecessary
.c_str()calls in exception handling.The
.c_str()calls are unnecessary when usingto_pg_msgas it can directly accept string objects.Apply this diff to remove the unnecessary conversions:
- *err_msg = to_pg_msg(ex.first.c_str()); - *log_msg = to_pg_msg(ex.second.c_str()); + *err_msg = to_pg_msg(ex.first); + *log_msg = to_pg_msg(ex.second);Likely invalid or redundant comment.
src/lineGraph/lineGraph_driver.cpp (1)
68-68: LGTM! String handling improvements are complete.All changes correctly implement the transition to
to_pg_msgwithout any unnecessary string conversions.Also applies to: 87-88, 109-109, 125-126, 131-132, 134-135, 140-141, 146-147
src/spanningTree/randomSpanningTree_driver.cpp (1)
64-64: LGTM! String handling improvements are complete.All changes correctly implement the transition to
to_pg_msgwithout any unnecessary string conversions.Also applies to: 84-85, 126-127, 132-133, 135-136, 141-142, 147-148
src/spanningTree/kruskal_driver.cpp (1)
61-61: LGTM! String handling improvements are complete.All changes correctly implement the transition to
to_pg_msgwithout any unnecessary string conversions.Also applies to: 95-96, 108-108, 122-123, 128-129, 131-132, 137-138, 143-144
src/spanningTree/prim_driver.cpp (1)
63-63: LGTM! String handling improvements are complete.All changes correctly implement the transition to
to_pg_msgwithout any unnecessary string conversions.Also applies to: 97-98, 110-110, 124-125, 130-131, 133-134, 139-140, 145-146
src/breadthFirstSearch/breadthFirstSearch_driver.cpp (1)
70-70: LGTM! Message handling updates are correct and consistent.The conversion from
pgr_msgtoto_pg_msgis properly implemented across all message assignments. The changes maintain the original error handling flow while potentially improving performance by eliminating unnecessary string conversions.Also applies to: 94-95, 120-120, 130-131, 136-137, 139-140, 145-146, 151-152
src/bdAstar/bdAstar_driver.cpp (1)
65-65: LGTM! Message handling updates are correct and consistent.The conversion from
pgr_msgtoto_pg_msgis properly implemented across all message assignments, following the same pattern as previously suggested.Also applies to: 86-87, 95-96, 121-121, 128-129, 134-135, 137-138, 143-144, 149-150
src/driving_distance/driving_distance_driver.cpp (1)
59-59: LGTM! Message handling updates are correct and consistent.The conversion from
pgr_msgtoto_pg_msgis properly implemented across all message assignments, following the same pattern as previously suggested.Also applies to: 83-84, 106-106, 127-128, 133-134, 136-137, 142-143, 148-149
src/astar/astar_driver.cpp (1)
64-64: LGTM! Message handling updates are correct and consistent.The conversion from
pgr_msgtoto_pg_msgis properly implemented across all message assignments, following the same pattern as previously suggested.Also applies to: 88-89, 99-100, 131-131, 138-139, 144-145, 147-148, 153-154, 159-160
src/dagShortestPath/dagShortestPath_driver.cpp (1)
80-80: LGTM! Message handling updates are correct and consistent.The conversion from
pgr_msgtoto_pg_msgis properly implemented across all message assignments, following the same pattern as previously suggested.Also applies to: 103-104, 114-115, 137-137, 144-145, 150-151, 153-154, 159-160, 165-166
src/ksp/ksp_driver.cpp (1)
67-67: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message handling paths, improving performance by eliminating unnecessary string conversions.Also applies to: 95-96, 104-105, 129-129, 142-143, 148-149, 151-152, 157-158, 163-164
src/bellman_ford/edwardMoore_driver.cpp (1)
82-82: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message handling paths, improving performance by eliminating unnecessary string conversions.Also applies to: 106-107, 115-116, 139-139, 146-147, 152-153, 155-156, 161-162, 167-168
src/max_flow/minCostMaxFlow_driver.cpp (1)
62-62: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message handling paths, improving performance by eliminating unnecessary string conversions.Also applies to: 83-84, 99-99, 107-108, 143-144, 149-150, 152-153, 158-159, 164-165
src/transitiveClosure/transitiveClosure_driver.cpp (1)
105-105: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message handling paths, improving performance by eliminating unnecessary string conversions.Also applies to: 123-124, 137-138, 143-144, 146-147, 152-153, 158-159
src/bellman_ford/bellman_ford_driver.cpp (1)
82-82: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message handling paths, improving performance by eliminating unnecessary string conversions.Also applies to: 106-107, 115-116, 139-139, 146-147, 152-153, 155-156, 161-162, 167-168
src/max_flow/max_flow_driver.cpp (1)
61-61: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message assignments, eliminating unnecessary string conversions and improving performance. The pattern is applied uniformly throughout the file.Also applies to: 82-83, 98-98, 107-107, 126-126, 148-149, 154-155, 160-161, 163-164, 169-170
src/bdDijkstra/bdDijkstra_driver.cpp (1)
98-98: LGTM! Consistent string handling improvements.The changes align with the codebase-wide effort to improve string handling by replacing
pgr_msgwithto_pg_msg. The modifications are applied consistently throughout the file.Also applies to: 119-120, 130-131, 154-154, 161-162, 167-168, 170-171, 176-177, 182-183
src/bellman_ford/bellman_ford_neg_driver.cpp (1)
84-84: LGTM! String handling improvements are consistent.The changes maintain consistency with the codebase-wide effort to improve string handling by replacing
pgr_msgwithto_pg_msg.Also applies to: 108-109, 120-121, 146-146, 153-154, 159-160, 162-163, 168-169, 174-175
src/dijkstra/dijkstraVia_driver.cpp (1)
106-106: LGTM! String handling improvements maintain consistency.The changes align with the codebase-wide effort to improve string handling by replacing
pgr_msgwithto_pg_msg. The modifications are applied uniformly throughout the file.Also applies to: 131-132, 167-167, 177-178, 183-184, 186-187, 192-193, 198-199
src/traversal/depthFirstSearch_driver.cpp (1)
97-97: LGTM! String handling improvements complete the consistent pattern.The changes complete the codebase-wide effort to improve string handling by replacing
pgr_msgwithto_pg_msg. The modifications maintain consistency with all other files.Also applies to: 122-123, 152-152, 163-164, 169-170, 172-173, 178-179, 184-185
src/max_flow/edge_disjoint_paths_driver.cpp (1)
86-86: LGTM! The changes improve string handling efficiency.The replacement of
pgr_msgwithto_pg_msgeliminates unnecessary string conversions and improves code clarity while maintaining the same functionality.Also applies to: 107-108, 117-118, 187-188, 193-194, 199-200, 202-203, 208-209
src/driving_distance/driving_distance_withPoints_driver.cpp (1)
70-70: LGTM! String handling improvements are consistently applied.The changes align with the PR objectives by replacing
pgr_msgwithto_pg_msg, improving performance by eliminating unnecessary string conversions.Also applies to: 102-103, 117-118, 151-151, 179-180, 185-186, 188-189, 194-195, 200-201
src/ksp/turnRestrictedPath_driver.cpp (2)
102-102: LGTM! String handling improvements are consistently applied.The changes align with the PR objectives by replacing
pgr_msgwithto_pg_msg, improving performance by eliminating unnecessary string conversions.Also applies to: 126-127, 208-209, 215-216, 221-222, 224-225, 230-231
210-211:⚠️ Potential issueFix potential null pointer dereference.
The null check
pgassert(*err_msg == NULL)is performed after using the pointer. This could lead to undefined behavior if the pointer is null.Move the assertion before any use of the pointer:
+ pgassert(*err_msg == NULL); *log_msg = to_pg_msg(log); *notice_msg = to_pg_msg(notice); - pgassert(*err_msg == NULL);Likely invalid or redundant comment.
src/withPoints/withPointsVia_driver.cpp (1)
114-114: LGTM! String handling improvements are consistently applied.The changes align with the PR objectives by replacing
pgr_msgwithto_pg_msg, improving performance by eliminating unnecessary string conversions.Also applies to: 144-145, 162-163, 206-206, 214-215, 220-221, 223-224, 229-230, 235-236
src/withPoints/withPoints_driver.cpp (1)
101-101: LGTM! String handling improvements are consistently applied.The changes align with the PR objectives by replacing
pgr_msgwithto_pg_msg, improving performance by eliminating unnecessary string conversions.Also applies to: 124-125, 140-141, 157-158, 208-208, 215-216, 221-222, 224-225, 230-231, 236-237
src/breadthFirstSearch/binaryBreadthFirstSearch_driver.cpp (1)
139-139: LGTM! Message handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message handling scenarios, eliminating unnecessary string conversions while maintaining the same functionality. The error handling pattern is well-structured and comprehensive.Also applies to: 165-166, 175-176, 187-187, 210-210, 221-221, 227-228, 230-231, 236-237, 242-243
src/ordering/cuthillMckeeOrdering_driver.cpp (1)
82-82: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message assignments, improving performance by eliminating unnecessary string conversions.Also applies to: 100-101, 117-117, 129-129, 132-132, 137-138, 140-141, 146-147, 152-153
src/lineGraph/lineGraphFull_driver.cpp (1)
78-78: LGTM! String handling improvements look good.The changes consistently replace
pgr_msgwithto_pg_msgacross all message assignments, improving performance by eliminating unnecessary string conversions.Also applies to: 96-97, 135-136, 141-142, 144-145, 150-151, 156-157
src/ksp/withPoints_ksp_driver.cpp (1)
73-73: LGTM! Successfully implemented previously suggested string handling improvements.The changes consistently replace
pgr_msgwithto_pg_msgacross all message assignments, as suggested in previous reviews.Also applies to: 102-103, 119-120, 133-134, 166-166, 183-183, 188-189, 194-195, 197-198, 203-204, 209-210
pgRouting#2725) (C++/performance) Removing passing c_str() to a function that takes std::string as argument * - rename pgr_msg -> to_pg_msg * -Adding overload of to_pg_msg * - Using the new functions * (docqueries) empty hint is not shown anymore (cherry picked from commit bf1d91e)
@pgRouting/admins
Summary by CodeRabbit
Release Notes
Code Improvement
.c_str()conversions when passing strings to message functions.Affected Components
No Functional Changes