From 49ff9e4fc3253a039b5436ff463b29fc2705d64a Mon Sep 17 00:00:00 2001 From: Simon Sticko Date: Mon, 4 Mar 2024 19:10:11 +0100 Subject: [PATCH] Fix unintended terminate in gmsh-API version of read_msh The gmsh-api version of GridIn::read_msh uses a throw statement without specifing an exception. The intention is cleary to throw any exception but the standard specifies that doing this outside of a catch block should lead to std::terminate being called. Set the boundary_id directly, without using throw, and add a test that covers this part of the function. --- source/grid/grid_in.cc | 93 +++++++++--------- tests/gmsh/gmsh_api_06.cc | 95 +++++++++++++++++++ .../gmsh_api_06.with_gmsh_with_api=on.output | 6 ++ 3 files changed, 149 insertions(+), 45 deletions(-) create mode 100644 tests/gmsh/gmsh_api_06.cc create mode 100644 tests/gmsh/gmsh_api_06.with_gmsh_with_api=on.output diff --git a/source/grid/grid_in.cc b/source/grid/grid_in.cc index 4521749cc4..71687f1984 100644 --- a/source/grid/grid_in.cc +++ b/source/grid/grid_in.cc @@ -2788,51 +2788,54 @@ GridIn::read_msh(const std::string &fname) std::string name; gmsh::model::getPhysicalName(entity_dim, physical_tag, name); if (!name.empty()) - try - { - std::map id_names; - Patterns::Tools::to_value(name, id_names); - bool throw_anyway = false; - bool found_boundary_id = false; - // If the above did not throw, we keep going, and retrieve - // all the information that we know how to translate. - for (const auto &it : id_names) - { - const auto &name = it.first; - const auto &id = it.second; - if (entity_dim == dim && name == "MaterialID") - { - boundary_id = static_cast(id); - found_boundary_id = true; - } - else if (entity_dim < dim && name == "BoundaryID") - { - boundary_id = static_cast(id); - found_boundary_id = true; - } - else if (name == "ManifoldID") - manifold_id = static_cast(id); - else - // We did not recognize one of the keys. We'll fall - // back to setting the boundary id to the physical tag - // after reading all strings. - throw_anyway = true; - } - // If we didn't find a BoundaryID:XX or MaterialID:XX, and - // something was found but not recognized, then we set the - // material id or boundary id in the catch block below, - // using directly the physical tag - if (throw_anyway && !found_boundary_id) - throw; - } - catch (...) - { - // When the above didn't work, we revert to the old - // behaviour: the physical tag itself is interpreted either - // as a material_id or a boundary_id, and no manifold id is - // known - boundary_id = physical_tag; - } + { + // Patterns::Tools::to_value throws an exception, if it can not + // convert name to a map from string to int. + try + { + std::map id_names; + Patterns::Tools::to_value(name, id_names); + bool found_unrecognized_tag = false; + bool found_boundary_id = false; + // If the above did not throw, we keep going, and retrieve + // all the information that we know how to translate. + for (const auto &it : id_names) + { + const auto &name = it.first; + const auto &id = it.second; + if (entity_dim == dim && name == "MaterialID") + { + boundary_id = static_cast(id); + found_boundary_id = true; + } + else if (entity_dim < dim && name == "BoundaryID") + { + boundary_id = static_cast(id); + found_boundary_id = true; + } + else if (name == "ManifoldID") + manifold_id = static_cast(id); + else + // We did not recognize one of the keys. We'll fall + // back to setting the boundary id to the physical tag + // after reading all strings. + found_unrecognized_tag = true; + } + // If we didn't find a BoundaryID:XX or MaterialID:XX, and + // something was found but not recognized, then we set the + // material id or boundary using the physical tag directly. + if (found_unrecognized_tag && !found_boundary_id) + boundary_id = physical_tag; + } + catch (...) + { + // When the above didn't work, we revert to the old + // behaviour: the physical tag itself is interpreted either + // as a material_id or a boundary_id, and no manifold id is + // known + boundary_id = physical_tag; + } + } } // Get the mesh elements for the entity (dim, tag): diff --git a/tests/gmsh/gmsh_api_06.cc b/tests/gmsh/gmsh_api_06.cc new file mode 100644 index 0000000000..b43e4fd7ed --- /dev/null +++ b/tests/gmsh/gmsh_api_06.cc @@ -0,0 +1,95 @@ +// ------------------------------------------------------------------------ +// +// SPDX-License-Identifier: LGPL-2.1-or-later +// Copyright (C) 2021 - 2023 by the deal.II authors +// +// This file is part of the deal.II library. +// +// Part of the source code is dual licensed under Apache-2.0 WITH +// LLVM-exception OR LGPL-2.1-or-later. Detailed license information +// governing the source code and code contributions can be found in +// LICENSE.md and CONTRIBUTING.md at the top level directory of deal.II. +// +// ------------------------------------------------------------------------ + +// Check that gmsh api correctly reads and writes a mesh with manifold +// information, in all coordinate dimensions + +#include +#include +#include +#include + +#include + +#include + +#include "../tests.h" + +/* + * Test that we can use the gmsh-API version of GridIn::read_msh, when the + * description of the "Physical name" does not contain MaterialID, BoundaryID, + * or ManifoldID. + * + * Create a hypercube triangulation, write it to file so that we get a valid + * msh-file. Read the filecontent back in and replace MaterialId with a string + * that will not be recognized. Write it back to the file and try to read the + * triangulation from the msh-file. + */ +template +void +test() +{ + // Create a mesh, write it to file. + Triangulation original_mesh; + const double left = 0.; + const double right = 1.; + const bool colorize = true; + GridGenerator::hyper_cube(original_mesh, left, right, colorize); + // Set materialID to something nonzero so that we + // get a physical name "MaterialID: 1" in the msh-file. + auto cell_on_original = original_mesh.begin_active(); + cell_on_original->set_material_id(17); + + const std::string mesh_filename = "output.msh"; + GridOut grid_out; + grid_out.write_msh(original_mesh, mesh_filename); + + // Read the file back and change the physical group description to be + // unrecognizable. + std::ifstream input(mesh_filename, std::ifstream::in); + std::stringstream buffert; + buffert << input.rdbuf(); + std::string filecontent = buffert.str(); + input.close(); + + boost::replace_all(filecontent, "MaterialID", "Unrecognizable"); + boost::replace_all(filecontent, "BoundaryID", "Unrecognizable"); + boost::replace_all(filecontent, "ManifoldID", "Unrecognizable"); + + // Write the mesh to file and read the triangulation back in. + std::ofstream output(mesh_filename); + output << filecontent; + output.close(); + + Triangulation triangulation_from_file; + GridIn grid_in(triangulation_from_file); + grid_in.read_msh(mesh_filename); + + // The function should set material_id and boundary_id based on the physical + // tag. Write these to log to check them. + const auto cell = triangulation_from_file.begin_active(); + deallog << "MaterialID = " << cell->material_id() << std::endl; + for (unsigned int f : cell->face_indices()) + deallog << "BoundaryID = " << cell->face(f)->boundary_id() << std::endl; +} + +int +main(int argc, char **argv) +{ + // gmsh might be build with mpi support enabled. + Utilities::MPI::MPI_InitFinalize mpi_initialization(argc, argv); + initlog(); + + test<2>(); +} diff --git a/tests/gmsh/gmsh_api_06.with_gmsh_with_api=on.output b/tests/gmsh/gmsh_api_06.with_gmsh_with_api=on.output new file mode 100644 index 0000000000..acc1037006 --- /dev/null +++ b/tests/gmsh/gmsh_api_06.with_gmsh_with_api=on.output @@ -0,0 +1,6 @@ + +DEAL::MaterialID = 4 +DEAL::BoundaryID = 0 +DEAL::BoundaryID = 1 +DEAL::BoundaryID = 2 +DEAL::BoundaryID = 3 -- 2.39.5