fix: use VJ actual terminus instead of Route destination in display_informations#4529
Conversation
…nformations The rel=terminus link in display_informations was pointing to the Route's static destination (Route.destination) instead of the VehicleJourney's actual last stop. This caused incorrect terminus information when a VJ's real terminus differs from the Route's configured destination. For example, on the RER C between Pont de l'Alma and Chemin d'Antony, the terminus was showing 'Gare de Thiais' (Route destination) instead of 'Massy-Palaiseau' (actual VJ terminus). After filling display_informations from the Route, we now override the terminus with the VJ's actual last stop area from stop_time_list. Co-Authored-By: ludovic.massenet@hove.com <ludovic.massenet@hove.com>
Original prompt from ludovic.massenet@hove.com
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| // The Route's destination may differ from the VJ's real terminus | ||
| // (e.g. a partial terminus on the route vs the actual end of the vehicle journey). | ||
| if (!vj_stoptimes->vj->stop_time_list.empty()) { | ||
| const auto* last_sa = vj_stoptimes->vj->stop_time_list.back().stop_point->stop_area; |
There was a problem hiding this comment.
🔴 Missing null check on stop_point before dereferencing it
At line 1627, stop_point is dereferenced without a null check: vj_stoptimes->vj->stop_time_list.back().stop_point->stop_area. The StopTime::stop_point field is a raw pointer initialized to nullptr (source/type/stop_time.h:65). Every other site in this file that accesses stop_time_list.back().stop_point guards it with a null check first — see is_principal_destination at source/type/pb_converter.cpp:170 and the origin/terminus fill at source/type/pb_converter.cpp:1743. If stop_point is null, this causes a null pointer dereference and crash.
| const auto* last_sa = vj_stoptimes->vj->stop_time_list.back().stop_point->stop_area; | |
| const auto* last_sp = vj_stoptimes->vj->stop_time_list.back().stop_point; | |
| const auto* last_sa = last_sp ? last_sp->stop_area : nullptr; |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Fixes the
rel=terminuslink indisplay_informationsfor/journeysresponses. Previously, the terminus was derived from the Route's staticdestinationproperty, which may not match the actual end of the VehicleJourney. This caused incorrect terminus info — e.g. on RER C between Pont de l'Alma and Chemin d'Antony, the terminus showed "Gare de Thiais" (Route destination) instead of "Massy-Palaiseau" (actual VJ terminus).After filling
PtDisplayInfofrom the Route, we now overrideuris.stop_areaand thepb_creator.terminusset with the VJ's real last stop area (vj->stop_time_list.back()).Review & Testing Checklist for Human
fill_pb_object(VjStopTimes*, PtDisplayInfo*)is also called for departure boards / stop schedules (not just journeys). Confirm the Route destination override is appropriate in those contexts too, or whether this override should be scoped only to the journey code path (e.g. infill_sectioninraptor_api.cppinstead).fill_with_creator(route, ...)) still insertsRoute.destinationintopb_creator.terminus. The new code then also inserts the VJ's last stop. When they differ, the response-levelterminusarray will now contain both entries. Verify whether the old Route destination should be removed from the set, or if having both is acceptable.stop_point:stop_time_list.back().stop_pointis dereferenced without a null check (onlystop_areais checked). Confirmstop_pointis guaranteed non-null in all code paths.Notes
simple_journeytest passes becausefill_missing_destinations()in the test builder sets Route.destination from the VJ's last stop, so Route destination == VJ terminus in test data. A test with an explicit Route.destination differing from the VJ's last stop would strengthen confidence.Link to Devin session: https://app.devin.ai/sessions/c10eca971f8d4078bd0146828c3f1ee6