From 6d7088adf5bf6a8b5da6db852965c26a7ba0d423 Mon Sep 17 00:00:00 2001 From: Xavier Del Campo Romero Date: Sat, 18 Jan 2025 06:52:56 +0100 Subject: [PATCH] OsgScenery.cpp: Fix undefined behaviour on delete SDTrack was taking the ownership for track, a pointer to tTrack allocated many layers above SDScenery::LoadScene. More specifically, it is allocated by either TrackBuildEx or TrackBuildv1, and both would allocate the tTrack instance via calloc(3) instead of C++'s new. Attempting to `delete` a pointer *not* allocated by a previous call to `new` is undefined behaviour according to the C++ standard. [1] In fact, SDScenery::LoadScene did not even attempt to modify the tTrack instance, so there was no need to take its ownership in the first place. [1]: https://en.cppreference.com/w/cpp/language/delete --- .../graphic/osggraph/Scenery/OsgScenery.cpp | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/modules/graphic/osggraph/Scenery/OsgScenery.cpp b/src/modules/graphic/osggraph/Scenery/OsgScenery.cpp index 8f7a5cf0d..f542c808c 100644 --- a/src/modules/graphic/osggraph/Scenery/OsgScenery.cpp +++ b/src/modules/graphic/osggraph/Scenery/OsgScenery.cpp @@ -43,8 +43,7 @@ SDScenery::SDScenery(void) : m_background(NULL), m_pit(NULL), m_tracklights(NULL), - _scenery(NULL), - SDTrack(NULL) + _scenery(NULL) { _max_visibility = 0; _nb_cloudlayer = 0; @@ -62,15 +61,12 @@ SDScenery::~SDScenery(void) delete m_background; delete m_pit; delete m_tracklights; - delete SDTrack; if(_scenery != NULL) { _scenery->removeChildren(0, _scenery->getNumChildren()); _scenery = NULL; } - - SDTrack = NULL; } void SDScenery::LoadScene(tTrack *track) @@ -85,7 +81,6 @@ void SDScenery::LoadScene(tTrack *track) m_pit = new SDPit; m_tracklights = new SDTrackLights; _scenery = new osg::Group; - SDTrack = track; // Load graphics options. LoadGraphicsOptions(); @@ -96,15 +91,15 @@ void SDScenery::LoadScene(tTrack *track) }//if grHandle /* Determine the world limits */ - grWrldX = (int)(SDTrack->max.x - SDTrack->min.x + 1); - grWrldY = (int)(SDTrack->max.y - SDTrack->min.y + 1); - grWrldZ = (int)(SDTrack->max.z - SDTrack->min.z + 1); + grWrldX = (int)(track->max.x - track->min.x + 1); + grWrldY = (int)(track->max.y - track->min.y + 1); + grWrldZ = (int)(track->max.z - track->min.z + 1); grWrldMaxSize = (int)(MAX(MAX(grWrldX, grWrldY), grWrldZ)); - if (strcmp(SDTrack->category, "speedway") == 0) + if (strcmp(track->category, "speedway") == 0) { _speedWay = true; - if (strcmp(SDTrack->subcategory, "long") == 0) + if (strcmp(track->subcategory, "long") == 0) _speedWayLong = true; else _speedWayLong = false; @@ -222,8 +217,6 @@ void SDScenery::ShutdownScene(void) //delete loader; _scenery->removeChildren(0, _scenery->getNumChildren()); _scenery = NULL; - - SDTrack = NULL; } bool SDScenery::LoadTrack(std::string& strTrack)