potential bug : delete faces that bound the region in function "int GRegion::delFace(GFace *face)" in GRegion.cpp
Hello, I found a potential bug in the function "int Gregion::delFace(GFace *face)" in GRegion.cpp. The code is as follows:
int GRegion::delFace(GFace *face)
{
const auto found = std::find(l_faces.begin(), l_faces.end(), face);
if(found != l_faces.end()) { l_faces.erase(found); }
const auto pos = std::distance(l_faces.begin(), found);
if(l_dirs.empty()) { return 0; }
if(l_dirs.size() < static_cast<std::size_t>(pos)) {
l_dirs.erase(std::prev(l_dirs.end()));
return 0;
}
const auto orientation = l_dirs.at(pos);
l_dirs.erase(std::next(l_dirs.begin(), pos));
return orientation;
}
In this code, if the parameter "face" is not found in "l_faces", then value "found" will point to "l_faces.end( )", then value "pos" will get the same value as "l_faces.size()". If the size of value "l_dirs" is the same as value "l_faces", then the code "l_dirs.at(pos)" will cause an out of range error.In addition, after the execution of the code "l_faces.erase(found)", "found" will become an invalid pointer, at this time, if execute the code "const auto pos = std::distance(l_faces.begin(), found);" in the debug environment under the msvc platform will report an error, but not in the release environment. The reason is that the debug environment checks the validity of the pointer while the release environment does not. So I modified the code a little bit, first, check whether the pointer "found" points to the variable "l_faces.end()", if so, return 0 directly; second, swap the code "const auto pos = std::distance(l_faces.begin(), found)" and "l_faces. erase(found)" to prevent errors in the debug environment of the msvc platform. The code is as follows:
int GRegion::delFace(GFace *face)
{
const auto found = std::find(l_faces.begin(), l_faces.end(), face);
if(found == l_faces.end()) { return 0; }
const auto pos = std::distance(l_faces.begin(), found);
l_faces.erase(found);
if(l_dirs.empty()) { return 0; }
if(l_dirs.size() < static_cast<std::size_t>(pos)) {
l_dirs.erase(std::prev(l_dirs.end()));
return 0;
}
const auto orientation = l_dirs.at(pos);
l_dirs.erase(std::next(l_dirs.begin(), pos));
return orientation;
}
I would be very happy if my modifications could solve some potential problems, thanks!