1
0
Fork 0

src/Viewer/viewmgr.*: added asserts, diagnostics and checking for out-of-range view numbers.

It seems that some aircraft sometimes incorrectly pass view indices rather
than numbers resulting in out-of-range access to FGViewMgr::views[]; See
flightgear-devel thread with subject "View crash (after loading errors)".

With this commit, if we are given an incorrect view number, we output a
diagnostic with SG_ALERT, assert fail, and cope with the problem. Arguably we
could do something more serious such as opening a popup or throw an exception.
This commit is contained in:
Julian Smith 2020-12-18 17:16:50 +00:00
parent 71c7659f98
commit 380ecdf8ba
2 changed files with 32 additions and 31 deletions

View file

@ -75,6 +75,15 @@ FGViewMgr::init ()
config_list = fgGetNode("/sim", true)->getChildren("view"); config_list = fgGetNode("/sim", true)->getChildren("view");
_current = fgGetInt("/sim/current-view/view-number"); _current = fgGetInt("/sim/current-view/view-number");
if (_current != 0 && (_current < 0 || _current >= (int) views.size())) {
SG_LOG(SG_VIEW, SG_ALERT,
"Invalid /sim/current-view/view-number=" << _current
<< ". views.size()=" << views.size()
<< ". Will assert false and use zero."
);
assert(0);
_current = 0;
}
for (unsigned int i = 0; i < config_list.size(); i++) { for (unsigned int i = 0; i < config_list.size(); i++) {
SGPropertyNode* n = config_list[i]; SGPropertyNode* n = config_list[i];
@ -185,23 +194,16 @@ void FGViewMgr::clear()
flightgear::View* flightgear::View*
FGViewMgr::get_current_view() FGViewMgr::get_current_view()
{ {
const auto numViews = static_cast<int>(views.size()); if (views.empty())
if ((_current >= 0) && (_current < numViews)) {
return views.at(_current);
} else {
return nullptr; return nullptr;
} assert(_current >= 0 && _current < (int) views.size());
return views[_current];
} }
const flightgear::View* const flightgear::View*
FGViewMgr::get_current_view() const FGViewMgr::get_current_view() const
{ {
const auto numViews = static_cast<int>(views.size()); return const_cast<FGViewMgr*>(this)->get_current_view();
if ((_current >= 0) && (_current < numViews)) {
return views.at(_current);
} else {
return nullptr;
}
} }
@ -294,7 +296,6 @@ void FGViewMgr::setCurrentViewIndex(int newview)
if (newview == _current) { if (newview == _current) {
return; return;
} }
// negative numbers -> set view with node index -newview // negative numbers -> set view with node index -newview
if (newview < 0) { if (newview < 0) {
for (int i = 0; i < (int)config_list.size(); i++) { for (int i = 0; i < (int)config_list.size(); i++) {
@ -302,25 +303,28 @@ void FGViewMgr::setCurrentViewIndex(int newview)
if (index == newview) if (index == newview)
newview = i; newview = i;
} }
if (newview < 0) if (newview < 0) {
SG_LOG(SG_VIEW, SG_ALERT,
"Failed to find -ve newview=" << newview
<< ". Will assert false and ignore."
);
assert(0);
return; return;
}
} }
if (newview < 0 || newview >= (int) views.size()) {
// not sure it really makes sense to be doing this wrapping logic SG_LOG(SG_VIEW, SG_ALERT, "Invalid newview=" << newview
// here, it could mask various strange inputs, But keeping for compat << ". views.size()=" << views.size()
// for now. << ". Will assert false and ignore."
const auto numViews = static_cast<int>(views.size()); );
if (newview < 0) { // wrap to last assert(0);
newview = numViews - 1; return;
} else if (newview >= numViews) { // wrap to zero
newview = 0;
} }
if (get_current_view()) { if (get_current_view()) {
get_current_view()->unbind(); get_current_view()->unbind();
} }
// set new view
_current = newview; _current = newview;
if (get_current_view()) { if (get_current_view()) {

View file

@ -75,19 +75,16 @@ public:
flightgear::View* next_view(); flightgear::View* next_view();
flightgear::View* prev_view(); flightgear::View* prev_view();
// Support for extra view windows. This only works if --compositer-viewer=1 was specified.
// //
// Calls SviewPush().
void view_push(); void view_push();
// Experimental. Only works if --compositer-viewer=1 was specified. Creates // These all end up calling SviewCreate().
// new window with clone of current view. As of 2020-09-03, the clone's
// scenery is not displayed correctly.
void clone_current_view(const SGPropertyNode* config); void clone_current_view(const SGPropertyNode* config);
//
void clone_last_pair(const SGPropertyNode* config); void clone_last_pair(const SGPropertyNode* config);
void clone_last_pair_double(const SGPropertyNode* config); void clone_last_pair_double(const SGPropertyNode* config);
void view_new(const SGPropertyNode* config); void view_new(const SGPropertyNode* config);
// setters // setters