From 8a8a44a4db0866aa6edca01d84b50cb113f98753 Mon Sep 17 00:00:00 2001 From: Kyle Bayes Date: Fri, 19 Jul 2024 05:40:54 -0700 Subject: [PATCH] Refactor numerical parsing in XML. PiperOrigin-RevId: 653979240 Change-Id: I4d39f4041b88396193eb35f0cc393141d64f6fc0 --- src/user/user_api.cc | 9 +- src/user/user_composite.cc | 40 +++---- src/user/user_mesh.cc | 9 +- src/user/user_util.cc | 118 +++++++++++++++++- src/user/user_util.h | 7 ++ src/xml/xml_native_reader.cc | 97 ++++++++------- src/xml/xml_native_writer.cc | 27 +++-- src/xml/xml_util.cc | 185 ++++++++--------------------- src/xml/xml_util.h | 14 --- test/user/user_util_test.cc | 101 +++++++++++++--- test/xml/xml_native_reader_test.cc | 20 +++- test/xml/xml_native_writer_test.cc | 8 +- test/xml/xml_util_test.cc | 57 --------- 13 files changed, 378 insertions(+), 314 deletions(-) delete mode 100644 test/xml/xml_util_test.cc diff --git a/src/user/user_api.cc b/src/user/user_api.cc index 8b7d449a..2c3b5aee 100644 --- a/src/user/user_api.cc +++ b/src/user/user_api.cc @@ -27,7 +27,12 @@ #include "user/user_model.h" #include "user/user_objects.h" #include "user/user_cache.h" -#include "xml/xml_util.h" + +namespace { + +using mujoco::user::StringToVector; + +} // namespace // global cache size in bytes (default 500MB) static constexpr std::size_t kGlobalCacheSize = 500 * (1 << 20); @@ -892,7 +897,7 @@ mjtByte mjs_setInStringVec(mjStringVec* dest, int i, const char* text) { // split text and copy into string array void mjs_setStringVec(mjStringVec* dest, const char* text) { std::vector* v = static_cast*>(dest); - *v = mjXUtil::String2Vector(text); + *v = StringToVector(text); } diff --git a/src/user/user_composite.cc b/src/user/user_composite.cc index 4d30be96..f76e2660 100644 --- a/src/user/user_composite.cc +++ b/src/user/user_composite.cc @@ -36,12 +36,11 @@ #include "user/user_model.h" #include "user/user_objects.h" #include "user/user_util.h" -#include "xml/xml_util.h" namespace { namespace mju = ::mujoco::util; -using std::vector; -using std::string; +using mujoco::user::VectorToString; +using mujoco::user::StringToVector; } // namespace // strncpy with 0, return false @@ -418,12 +417,12 @@ bool mjCComposite::MakeParticle(mjCModel* model, mjsBody* body, char* error, int } } } - mjXUtil::Vector2String(userface, face); + userface = VectorToString(face); } else { dim = 2; // can only load a surface for now - face = mjXUtil::String2Vector(userface); + face = StringToVector(userface); for (int i=0; i(userface); + face = StringToVector(userface); for (int j=0; j skinface; - skinface = mjXUtil::String2Vector(userface); + skinface = StringToVector(userface); int nvert = uservert.size()/3; for (int j=0; j<2; j++) { @@ -2034,9 +2033,9 @@ void mjCComposite::MakeSkin2Subgrid(mjCModel* model, mjtNum inflate) { // add skin to 3D void mjCComposite::MakeSkin3(mjCModel* model) { int vcnt = 0; - std::map vmap; + std::map vmap; char txt[100], cnt0[10], cnt1[10], cnt2[10]; - string fmt; + std::string fmt; // string counts mju::sprintf_arr(cnt0, "%d", count[0]-1); @@ -2056,17 +2055,17 @@ void mjCComposite::MakeSkin3(mjCModel* model) { if (type==mjCOMPTYPE_BOX || type==mjCOMPTYPE_PARTICLE) { // z-faces MakeSkin3Box(skin, count[0], count[1], 1, vcnt, "%sB%d_%d_0"); - fmt = "%sB%d_%d_" + string(cnt2); + fmt = "%sB%d_%d_" + std::string(cnt2); MakeSkin3Box(skin, count[0], count[1], 0, vcnt, fmt.c_str()); // y-faces MakeSkin3Box(skin, count[0], count[2], 0, vcnt, "%sB%d_0_%d"); - fmt = "%sB%d_" + string(cnt1) + "_%d"; + fmt = "%sB%d_" + std::string(cnt1) + "_%d"; MakeSkin3Box(skin, count[0], count[2], 1, vcnt, fmt.c_str()); // x-faces MakeSkin3Box(skin, count[1], count[2], 1, vcnt, "%sB0_%d_%d"); - fmt = "%sB" + string(cnt0) + "_%d_%d"; + fmt = "%sB" + std::string(cnt0) + "_%d_%d"; MakeSkin3Box(skin, count[1], count[2], 0, vcnt, fmt.c_str()); } @@ -2112,17 +2111,17 @@ void mjCComposite::MakeSkin3(mjCModel* model) { // y-faces MakeSkin3Smooth(skin, count[0], count[2], 0, vmap, "%sB%d_0_%d"); - fmt = "%sB%d_" + string(cnt1) + "_%d"; + fmt = "%sB%d_" + std::string(cnt1) + "_%d"; MakeSkin3Smooth(skin, count[0], count[2], 1, vmap, fmt.c_str()); // x-faces MakeSkin3Smooth(skin, count[1], count[2], 1, vmap, "%sB0_%d_%d"); - fmt = "%sB" + string(cnt0) + "_%d_%d"; + fmt = "%sB" + std::string(cnt0) + "_%d_%d"; MakeSkin3Smooth(skin, count[1], count[2], 0, vmap, fmt.c_str()); // z-faces, boxy-type MakeSkin3Box(skin, count[0], count[1], 1, vcnt, "%sB%d_%d_0"); - fmt = "%sB%d_%d_" + string(cnt2); + fmt = "%sB%d_%d_" + std::string(cnt2); MakeSkin3Box(skin, count[0], count[1], 0, vcnt, fmt.c_str()); } @@ -2172,17 +2171,17 @@ void mjCComposite::MakeSkin3(mjCModel* model) { // z-faces MakeSkin3Smooth(skin, count[0], count[1], 1, vmap, "%sB%d_%d_0"); - fmt = "%sB%d_%d_" + string(cnt2); + fmt = "%sB%d_%d_" + std::string(cnt2); MakeSkin3Smooth(skin, count[0], count[1], 0, vmap, fmt.c_str()); // y-faces MakeSkin3Smooth(skin, count[0], count[2], 0, vmap, "%sB%d_0_%d"); - fmt = "%sB%d_" + string(cnt1) + "_%d"; + fmt = "%sB%d_" + std::string(cnt1) + "_%d"; MakeSkin3Smooth(skin, count[0], count[2], 1, vmap, fmt.c_str()); // x-faces MakeSkin3Smooth(skin, count[1], count[2], 1, vmap, "%sB0_%d_%d"); - fmt = "%sB" + string(cnt0) + "_%d_%d"; + fmt = "%sB" + std::string(cnt0) + "_%d_%d"; MakeSkin3Smooth(skin, count[1], count[2], 0, vmap, fmt.c_str()); } @@ -2248,7 +2247,8 @@ void mjCComposite::MakeSkin3Box(mjsSkin* skin, int c0, int c1, int side, // make one face of 3D skin, smooth void mjCComposite::MakeSkin3Smooth(mjsSkin* skin, int c0, int c1, int side, - const std::map& vmap, const char* format) { + const std::map& vmap, + const char* format) { char txt00[100], txt01[100], txt10[100], txt11[100]; // loop over bodies/vertices of specified face diff --git a/src/user/user_mesh.cc b/src/user/user_mesh.cc index b15c75fd..1b2d1022 100644 --- a/src/user/user_mesh.cc +++ b/src/user/user_mesh.cc @@ -62,13 +62,16 @@ #include "user/user_objects.h" #include "user/user_resource.h" #include "user/user_util.h" -#include "xml/xml_util.h" #include extern "C" { #include "qhull_ra.h" } +namespace { + using mujoco::user::VectorToString; +} // namespace + // compute triangle area, surface normal, center static double _triangle(double* normal, double* center, const float* v1, const float* v2, const float* v3) { @@ -2627,8 +2630,8 @@ void mjCFlex::Compile(const mjVFS* vfs) { // add plugins std::string userface, useredge; - mjXUtil::Vector2String(userface, elem_); - mjXUtil::Vector2String(useredge, edgeidx); + userface = VectorToString(elem_); + useredge = VectorToString(edgeidx); for (const auto& vbodyid : vertbodyid) { if (model->Bodies()[vbodyid]->plugin.instance) { diff --git a/src/user/user_util.cc b/src/user/user_util.cc index 92ce36c4..842229e8 100644 --- a/src/user/user_util.cc +++ b/src/user/user_util.cc @@ -16,11 +16,13 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -32,6 +34,16 @@ #include #include "engine/engine_crossplatform.h" + +// workaround with locale bug on some MacOS machines +#if defined (__APPLE__) && defined (__MACH__) +#include +#include + +#define strtof(X, Y) strtof_l((X), (Y), _c_locale) +#define strtod(X, Y) strtod_l((X), (Y), _c_locale) +#endif + // check if numeric variable is defined bool mjuu_defined(double num) { return !std::isnan(num); @@ -1137,5 +1149,109 @@ std::vector FileToMemory(const char* filename) { return buffer; } -} // namespace mujoco::user +// convert vector to string separating elements by whitespace +template std::string VectorToString(const std::vector& v) { + std::stringstream ss; + for (const T& t : v) { + ss << t << " "; + } + + std::string s = ss.str(); + if (!s.empty()) s.pop_back(); // remove trailing space + return s; +} + +template std::string VectorToString(const std::vector& v); +template std::string VectorToString(const std::vector& v); +template std::string VectorToString(const std::vector& v); +template std::string VectorToString(const std::vector& v); + +namespace { + +template T StrToNum(char* str, char** c); + +template<> int StrToNum(char* str, char** c) { + long n = std::strtol(str, c, 10); + if (n < INT_MIN || n > INT_MAX) errno = ERANGE; + return n; +} + +template<> float StrToNum(char* str, char** c) { + float f = strtof(str, c); + if (std::isnan(f)) errno = EDOM; + return f; +} + +template<> double StrToNum(char* str, char** c) { + double d = strtod(str, c); + if (std::isnan(d)) errno = EDOM; + return d; +} + +template<> unsigned char StrToNum(char* str, char** c) { + long n = std::strtol(str, c, 10); + if (n < 0 || n > UCHAR_MAX) errno = ERANGE; + return n; +} + +inline bool IsNullOrSpace(char* c) { + return std::isspace(static_cast(*c)) || *c == '\0'; +} + +inline char* SkipSpace(char* c) { + for (; *c != '\0'; c++) { + if (!IsNullOrSpace(c)) { + break; + } + } + return c; +} +} // namespace + +template std::vector StringToVector(char* cs) { + std::vector v; + char* ch = cs; + + errno = 0; + // reserve worst case + v.reserve((std::strlen(cs) >> 1) + 1); + + for (;;) { + cs = SkipSpace(ch); // skip leading spaces + if (*cs == '\0') break; // end of string + T num = StrToNum(cs, &ch); // parse number + if (!IsNullOrSpace(ch)) errno = EINVAL; // invalid separator + if (cs == ch) errno = EINVAL; // failed to parse number + if (errno && errno != EDOM) break; // NaNs are quietly ignored + v.push_back(num); + } + + v.shrink_to_fit(); + return v; +} + +template<> std::vector StringToVector(const std::string& s) { + std::vector v; + std::stringstream ss(s); + std::string word; + while (ss >> word) { + v.push_back(word); + } + return v; +} + +template std::vector StringToVector(char* cs); +template std::vector StringToVector(char* cs); +template std::vector StringToVector(char* cs); + + +template std::vector StringToVector(const std::string& s) { + return StringToVector(const_cast(s.c_str())); +} +template std::vector StringToVector(const std::string& s); +template std::vector StringToVector(const std::string& s); +template std::vector StringToVector(const std::string& s); +template std::vector StringToVector(const std::string& s); + +} // namespace mujoco::user diff --git a/src/user/user_util.h b/src/user/user_util.h index 1bce5010..8a95d61e 100644 --- a/src/user/user_util.h +++ b/src/user/user_util.h @@ -235,6 +235,13 @@ class FilePath { // read file into memory buffer std::vector FileToMemory(const char* filename); +// convert vector to string separating elements by whitespace +template std::string VectorToString(const std::vector& v); + +// convert string to vector +template std::vector StringToVector(char *cs); +template std::vector StringToVector(const std::string& s); + } // namespace mujoco::user // strip path from filename diff --git a/src/xml/xml_native_reader.cc b/src/xml/xml_native_reader.cc index bd7f2ca3..62b5b50d 100644 --- a/src/xml/xml_native_reader.cc +++ b/src/xml/xml_native_reader.cc @@ -1317,17 +1317,17 @@ void mjXReader::OneFlex(XMLElement* elem, mjsFlex* pflex) { if (ReadAttrTxt(elem, "body", text, true)) { mjs_setStringVec(pflex->vertbody, text.c_str()); } - if (ReadAttrTxt(elem, "vertex", text)) { - std::vector vert = String2Vector(text); - mjs_setDouble(pflex->vert, vert.data(), vert.size()); + auto vert = ReadAttrVec(elem, "vertex"); + if (vert.has_value()) { + mjs_setDouble(pflex->vert, vert->data(), vert->size()); } - if (ReadAttrTxt(elem, "element", text, true)) { - std::vector elem = String2Vector(text); - mjs_setInt(pflex->elem, elem.data(), elem.size()); + auto element = ReadAttrVec(elem, "element", true); + if (element.has_value()) { + mjs_setInt(pflex->elem, element->data(), element->size()); } - if (ReadAttrTxt(elem, "texcoord", text)) { - std::vector texcoord = String2Vector(text); - mjs_setFloat(pflex->texcoord, texcoord.data(), texcoord.size()); + auto texcoord = ReadAttrVec(elem, "texcoord"); + if (texcoord.has_value()) { + mjs_setFloat(pflex->texcoord, texcoord->data(), texcoord->size()); } // contact subelement @@ -1459,21 +1459,21 @@ void mjXReader::OneSkin(XMLElement* elem, mjsSkin* pskin) { ReadAttr(elem, "inflate", 1, &pskin->inflate, text); // read vertex data - if (ReadAttrTxt(elem, "vertex", text)) { - std::vector vert = String2Vector(text); - mjs_setFloat(pskin->vert, vert.data(), vert.size()); + auto vertex = ReadAttrVec(elem, "vertex"); + if (vertex.has_value()) { + mjs_setFloat(pskin->vert, vertex->data(), vertex->size()); } // read texcoord data - if (ReadAttrTxt(elem, "texcoord", text)) { - std::vector texcoord = String2Vector(text); - mjs_setFloat(pskin->texcoord, texcoord.data(), texcoord.size()); + auto texcoord = ReadAttrVec(elem, "texcoord"); + if (texcoord.has_value()) { + mjs_setFloat(pskin->texcoord, texcoord->data(), texcoord->size()); } // read user face data - if (ReadAttrTxt(elem, "face", text)) { - std::vector face = String2Vector(text); - mjs_setInt(pskin->face, face.data(), face.size()); + auto face = ReadAttrVec(elem, "face"); + if (face.has_value()) { + mjs_setInt(pskin->face, face->data(), face->size()); } // read bones @@ -1500,14 +1500,16 @@ void mjXReader::OneSkin(XMLElement* elem, mjsSkin* pskin) { bindquat.push_back(data[3]); // read vertid - ReadAttrTxt(bone, "vertid", text, true); - vector tempid = String2Vector(text); - mjs_appendIntVec(pskin->vertid, tempid.data(), tempid.size()); + auto tempid = ReadAttrVec(bone, "vertid", true); + if (tempid.has_value()) { + mjs_appendIntVec(pskin->vertid, tempid->data(), tempid->size()); + } // read vertweight - ReadAttrTxt(bone, "vertweight", text, true); - vector tempweight = String2Vector(text); - mjs_appendFloatVec(pskin->vertweight, tempweight.data(), tempweight.size()); + auto tempweight = ReadAttrVec(bone, "vertweight", true); + if (tempweight.has_value()) { + mjs_appendFloatVec(pskin->vertweight, tempweight->data(), tempweight->size()); + } // advance to next bone bone = NextSiblingElement(bone, "bone"); @@ -2287,8 +2289,9 @@ void mjXReader::OneComposite(XMLElement* elem, mjsBody* pbody, mjsDefault* def) ReadAttrTxt(elem, "curve", curves); ReadAttrTxt(elem, "initial", comp.initial); ReadAttr(elem, "size", 3, comp.size, text, false, false); - if (ReadAttrTxt(elem, "vertex", text)) { - comp.uservert = String2Vector(text); + auto uservert = ReadAttrVec(elem, "vertex"); + if (uservert.has_value()) { + comp.uservert = std::move(uservert.value()); } // shell @@ -2530,14 +2533,17 @@ void mjXReader::OneFlexcomp(XMLElement* elem, mjsBody* pbody) { if (MapValue(elem, "rigid", &n, bool_map, 2)) { fcomp.rigid = (n==1); } - if (ReadAttrTxt(elem, "point", text)){ - fcomp.point = String2Vector(text); + auto point = ReadAttrVec(elem, "point"); + if (point.has_value()) { + fcomp.point = std::move(point.value()); } - if (ReadAttrTxt(elem, "element", text)){ - fcomp.element = String2Vector(text); + auto element = ReadAttrVec(elem, "element"); + if (element.has_value()) { + fcomp.element = std::move(element.value()); } - if (ReadAttrTxt(elem, "texcoord", text)) { - fcomp.texcoord = String2Vector(text); + auto texcoord = ReadAttrVec(elem, "texcoord"); + if (texcoord.has_value()) { + fcomp.texcoord = std::move(texcoord.value()); } // edge @@ -2575,23 +2581,22 @@ void mjXReader::OneFlexcomp(XMLElement* elem, mjsBody* pbody) { // pin XMLElement* epin = FirstChildElement(elem, "pin"); while (epin) { - // accumulate id, coord, range - if (ReadAttrTxt(epin, "id", text)) { - vector v = String2Vector(text); - fcomp.pinid.insert(fcomp.pinid.end(), v.begin(), v.end()); + auto id = ReadAttrVec(epin, "id"); + if (id.has_value()) { + fcomp.pinid.insert(fcomp.pinid.end(), id->begin(), id->end()); } - if (ReadAttrTxt(epin, "range", text)) { - vector v = String2Vector(text); - fcomp.pinrange.insert(fcomp.pinrange.end(), v.begin(), v.end()); + auto range = ReadAttrVec(epin, "range"); + if (range.has_value()) { + fcomp.pinrange.insert(fcomp.pinrange.end(), range->begin(), range->end()); } - if (ReadAttrTxt(epin, "grid", text)) { - - vector v = String2Vector(text); - fcomp.pingrid.insert(fcomp.pingrid.end(), v.begin(), v.end()); + auto grid = ReadAttrVec(epin, "grid"); + if (grid.has_value()) { + fcomp.pingrid.insert(fcomp.pingrid.end(), grid->begin(), grid->end()); } - if (ReadAttrTxt(epin, "gridrange", text)) { - vector v = String2Vector(text); - fcomp.pingridrange.insert(fcomp.pingridrange.end(), v.begin(), v.end()); + auto gridrange = ReadAttrVec(epin, "gridrange"); + if (gridrange.has_value()) { + fcomp.pingridrange.insert(fcomp.pingridrange.end(), + gridrange->begin(), gridrange->end()); } // advance diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index 99af4293..d6e4a9ea 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -44,6 +44,7 @@ using std::string; using tinyxml2::XMLComment; using tinyxml2::XMLDocument; using tinyxml2::XMLElement; +using mujoco::user::VectorToString; } // namespace @@ -142,19 +143,19 @@ void mjXWriter::OneFlex(XMLElement* elem, const mjCFlex* pflex) { // data vectors if (!pflex->get_vertbody().empty()) { - Vector2String(text, pflex->get_vertbody()); + text = VectorToString(pflex->get_vertbody()); WriteAttrTxt(elem, "body", text); } if (!pflex->get_vert().empty()) { - Vector2String(text, pflex->get_vert()); + text = VectorToString(pflex->get_vert()); WriteAttrTxt(elem, "vertex", text); } if (!pflex->get_elem().empty()) { - Vector2String(text, pflex->get_elem()); + text = VectorToString(pflex->get_elem()); WriteAttrTxt(elem, "element", text); } if (!pflex->get_texcoord().empty()) { - Vector2String(text, pflex->get_texcoord()); + text = VectorToString(pflex->get_texcoord()); WriteAttrTxt(elem, "texcoord", text); } @@ -207,25 +208,25 @@ void mjXWriter::OneMesh(XMLElement* elem, const mjCMesh* pmesh, mjCDef* def) { // write vertex data if (!pmesh->get_uservert().empty()) { - Vector2String(text, pmesh->get_uservert()); + text = VectorToString(pmesh->get_uservert()); WriteAttrTxt(elem, "vertex", text); } // write normal data if (!pmesh->get_usernormal().empty()) { - Vector2String(text, pmesh->get_usernormal()); + text = VectorToString(pmesh->get_usernormal()); WriteAttrTxt(elem, "normal", text); } // write texcoord data if (!pmesh->get_usertexcoord().empty()) { - Vector2String(text, pmesh->get_usertexcoord()); + text = VectorToString(pmesh->get_usertexcoord()); WriteAttrTxt(elem, "texcoord", text); } // write face data if (!pmesh->get_userface().empty()) { - Vector2String(text, pmesh->get_userface()); + text = VectorToString(pmesh->get_userface()); WriteAttrTxt(elem, "face", text); } } @@ -257,17 +258,17 @@ void mjXWriter::OneSkin(XMLElement* elem, const mjCSkin* pskin) { // write data if no file if (pskin->get_file().empty()) { // mesh vert - Vector2String(text, pskin->get_vert()); + text = VectorToString(pskin->get_vert()); WriteAttrTxt(elem, "vertex", text); // mesh texcoord if (!pskin->get_texcoord().empty()) { - Vector2String(text, pskin->get_texcoord()); + text = VectorToString(pskin->get_texcoord()); WriteAttrTxt(elem, "texcoord", text); } // mesh face - Vector2String(text, pskin->get_face()); + text = VectorToString(pskin->get_face()); WriteAttrTxt(elem, "face", text); // bones @@ -281,11 +282,11 @@ void mjXWriter::OneSkin(XMLElement* elem, const mjCSkin* pskin) { WriteAttr(bone, "bindquat", 4, pskin->get_bindquat().data()+4*i); // write vertid - Vector2String(text, pskin->get_vertid()[i]); + text = VectorToString(pskin->get_vertid()[i]); WriteAttrTxt(bone, "vertid", text); // write vertweight - Vector2String(text, pskin->get_vertweight()[i]); + text = VectorToString(pskin->get_vertweight()[i]); WriteAttrTxt(bone, "vertweight", text); } } diff --git a/src/xml/xml_util.cc b/src/xml/xml_util.cc index 038dba05..a0740cf0 100644 --- a/src/xml/xml_util.cc +++ b/src/xml/xml_util.cc @@ -43,10 +43,6 @@ namespace { -using std::size_t; -using std::string; -using std::stringstream; -using std::vector; using tinyxml2::XMLAttribute; using tinyxml2::XMLElement; @@ -238,7 +234,7 @@ mjXSchema::mjXSchema(const char* schema[][mjXATTRNUM], unsigned nrow) { // get pointer to error message -string mjXSchema::GetError() { +std::string mjXSchema::GetError() { return error; } @@ -256,7 +252,7 @@ static void printspace(std::stringstream& str, int n, const char* space) { // print schema as text void mjXSchema::Print(std::stringstream& str, int level) const { // replace body with (world)body - string name1 = (name_ == "body") ? "(world)body" : name_; + std::string name1 = (name_ == "body") ? "(world)body" : name_; // space, name, type printspace(str, 3*level, " "); @@ -290,7 +286,7 @@ void mjXSchema::Print(std::stringstream& str, int level) const { // print schema as HTML table void mjXSchema::PrintHTML(std::stringstream& str, int level, bool pad) const { // replace body with (world)body - string name1 = (name_ == "body" ? "(world)body" : name_); + std::string name1 = (name_ == "body" ? "(world)body" : name_); // open table if (level==0) { @@ -377,7 +373,7 @@ XMLElement* mjXSchema::Check(XMLElement* elem, int level) { const XMLAttribute* attribute = elem->FirstAttribute(); for (; attribute != nullptr; attribute = attribute->Next()) { if (attr_.find(attribute->Name()) == attr_.end()) { - error = "unrecognized attribute: '" + string(attribute->Name()) + "'"; + error = "unrecognized attribute: '" + std::string(attribute->Name()) + "'"; return elem; } } @@ -541,7 +537,7 @@ template bool mjXUtil::SameVector(const unsigned char* vec1, const unsigned char // find string in map, return corresponding integer (-1: not found) -int mjXUtil::FindKey(const mjMap* map, int mapsz, string key) { +int mjXUtil::FindKey(const mjMap* map, int mapsz, std::string key) { for (int i=0; i std::optional> mjXUtil::ReadAttrVec(XMLElement* elem, const char* attr, bool required) { - std::vector vec; - ReadAttrValues(elem, attr, [&](int i, T num) { vec.push_back(num); }); - if (!vec.size()) { + std::vector v; + const char* raw_cstr = elem->Attribute(attr); + if (raw_cstr) { + v = mujoco::user::StringToVector(raw_cstr); + if (errno == EDOM) { + mju_warning("XML contains a 'NaN'. Please check it carefully."); + } else if (errno == ERANGE) { + throw mjXError(elem, "number is too large in attribute '%s'", attr); + } else if (errno == EINVAL) { + throw mjXError(elem, "bad format in attribute '%s'", attr); + } else if (errno != 0) { + throw mjXError(elem, "unknown error in attribute '%s'", attr); + } + } + + if (!v.size()) { if (required) { throw mjXError(elem, "required attribute missing: '%s'", attr); } else { @@ -580,7 +589,7 @@ std::optional> mjXUtil::ReadAttrVec(XMLElement* elem, const char* } } - return vec; + return v; } template std::optional> @@ -651,7 +660,7 @@ mjXUtil::ReadAttrNum(XMLElement* elem, const char* attr, bool required); // return number of elements found template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, - T* data, string& text, bool required, bool exact) { + T* data, std::string& text, bool required, bool exact) { auto maybe_vec = ReadAttrVec(elem, attr, required); if (!maybe_vec.has_value()) { return 0; @@ -672,21 +681,21 @@ int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, } template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, int len, - double* data, string& text, bool required, bool exact); + double* data, std::string& text, bool required, bool exact); template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, int len, - float* data, string& text, bool required, bool exact); + float* data, std::string& text, bool required, bool exact); template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, int len, - int* data, string& text, bool required, bool exact); + int* data, std::string& text, bool required, bool exact); template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, int len, - unsigned char* data, string& text, bool required, + unsigned char* data, std::string& text, bool required, bool exact); // read quaternion attribute // throw error if identically zero -int mjXUtil::ReadQuat(XMLElement* elem, const char* attr, double* data, string& text, +int mjXUtil::ReadQuat(XMLElement* elem, const char* attr, double* data, std::string& text, bool required) { ReadAttr(elem, attr, /*len=*/4, data, text, required, /*exact=*/true); @@ -700,7 +709,7 @@ int mjXUtil::ReadQuat(XMLElement* elem, const char* attr, double* data, string& // read DOUBLE array into C++ vector, return number read int mjXUtil::ReadVector(XMLElement* elem, const char* attr, - vector& vec, string& text, bool required) { + std::vector& vec, std::string& text, bool required) { auto maybe_vec = ReadAttrVec(elem, attr, required); if (!maybe_vec.has_value()) { return 0; @@ -713,7 +722,8 @@ int mjXUtil::ReadVector(XMLElement* elem, const char* attr, // read text field -bool mjXUtil::ReadAttrTxt(tinyxml2::XMLElement* elem, const char* attr, string& text, bool required) { +bool mjXUtil::ReadAttrTxt(tinyxml2::XMLElement* elem, const char* attr, + std::string& text, bool required) { auto maybe_str = ReadAttrStr(elem, attr, required); if (!maybe_str.has_value()) { return false; @@ -735,92 +745,9 @@ bool mjXUtil::ReadAttrInt(XMLElement* elem, const char* attr, int* data, bool re } - -template<> int mjXUtil::StrToNum(char* str, char** c) { - return std::strtol(str, c, 10); -} - -template<> float mjXUtil::StrToNum(char* str, char** c) { - return std::strtof(str, c); -} - -template<> double mjXUtil::StrToNum(char* str, char** c) { - return std::strtod(str, c); -} - -template -std::vector mjXUtil::String2Vector(const std::string& s) { - errno = 0; - std::vector v; - char* cs = (char*) s.c_str(); - char* ch = cs; - - // reserve worst case - v.reserve((s.size() >> 1) + 1); - - for (;;) { - cs = ch; - T num = StrToNum(cs, &ch); - if (cs == ch) break; - if (errno) break; - v.push_back(num); - } - - v.shrink_to_fit(); - return v; -} - -template std::vector mjXUtil::String2Vector(const std::string& s); -template std::vector mjXUtil::String2Vector(const std::string& s); -template std::vector mjXUtil::String2Vector(const std::string& s); -template<> -std::vector mjXUtil::String2Vector(const std::string& s) { - std::vector v; - std::stringstream ss(s); - std::string word; - while (ss >> word) { - v.push_back(word); - } - return v; -} - - - // write vector to string -void mjXUtil::Vector2String(string& txt, const vector& vec) { - stringstream strm; - - for (size_t i=0; i0) { - strm << " "; - } - strm << vec[i]; - } - - txt = strm.str(); -} - - - -// write vector to string -void mjXUtil::Vector2String(string& txt, const vector& vec) { - stringstream strm; - - for (size_t i=0; i0) { - strm << " "; - } - strm << vec[i]; - } - - txt = strm.str(); -} - - - -// write vector to string -void mjXUtil::Vector2String(string& txt, const vector& vec, int ncol) { - stringstream strm; +void mjXUtil::Vector2String(std::string& txt, const std::vector& vec, int ncol) { + std::stringstream strm; for (size_t i=0; i& vec, int ncol) { txt = strm.str(); } - - -// write vector to string -void mjXUtil::Vector2String(string& txt, const vector& vec) { - stringstream strm; - - for (size_t i=0; i0) { - strm << " "; - } - strm << vec[i]; - } - - txt = strm.str(); -} - - - // find subelement with given name, make sure it is unique -XMLElement* mjXUtil::FindSubElem(XMLElement* elem, string name, bool required) { +XMLElement* mjXUtil::FindSubElem(XMLElement* elem, std::string name, bool required) { XMLElement* subelem = 0; XMLElement* iter = elem->FirstChildElement(); @@ -924,7 +833,7 @@ static int Round(double x) { // write attribute template -void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, const T* data, const T* def, +void mjXUtil::WriteAttr(XMLElement* elem, std::string name, int n, const T* data, const T* def, bool trim) { // make sure all are defined if constexpr (std::is_floating_point_v) { @@ -948,7 +857,7 @@ void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, const T* data, con } // increase precision for testing - stringstream stream; + std::stringstream stream; stream.precision(mujoco::_mjPRIVATE__get_xml_precision()); // process all numbers @@ -972,22 +881,22 @@ void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, const T* data, con } -template void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, +template void mjXUtil::WriteAttr(XMLElement* elem, std::string name, int n, const double* data, const double* def, bool trim); -template void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, +template void mjXUtil::WriteAttr(XMLElement* elem, std::string name, int n, const float* data, const float* def, bool trim); -template void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, +template void mjXUtil::WriteAttr(XMLElement* elem, std::string name, int n, const int* data, const int* def, bool trim); -template void mjXUtil::WriteAttr(XMLElement* elem, string name, int n, +template void mjXUtil::WriteAttr(XMLElement* elem, std::string name, int n, const unsigned char* data, const unsigned char* def, bool trim); // write vector attribute, default = zero array -void mjXUtil::WriteVector(XMLElement* elem, string name, const vector& vec) { +void mjXUtil::WriteVector(XMLElement* elem, std::string name, const std::vector& vec) { // proceed only if non-zero found bool ok = false; for (size_t i=0; i& v // write vector attribute, default with same size -void mjXUtil::WriteVector(XMLElement* elem, string name, const vector& vec, - const vector& def) { +void mjXUtil::WriteVector(XMLElement* elem, std::string name, const std::vector& vec, + const std::vector& def) { // proceed only if non-zero found bool ok = false; for (size_t i=0; i& v // write attribute- string -void mjXUtil::WriteAttrTxt(XMLElement* elem, string name, string value) { +void mjXUtil::WriteAttrTxt(XMLElement* elem, std::string name, std::string value) { // skip if value is empty if (value.empty()) { return; @@ -1039,7 +948,7 @@ void mjXUtil::WriteAttrTxt(XMLElement* elem, string name, string value) { // write attribute- single int -void mjXUtil::WriteAttrInt(XMLElement* elem, string name, int data, int def) { +void mjXUtil::WriteAttrInt(XMLElement* elem, std::string name, int data, int def) { // skip default if (data==def) { return; @@ -1051,7 +960,7 @@ void mjXUtil::WriteAttrInt(XMLElement* elem, string name, int data, int def) { // write attribute- keyword -void mjXUtil::WriteAttrKey(XMLElement* elem, string name, +void mjXUtil::WriteAttrKey(XMLElement* elem, std::string name, const mjMap* map, int mapsz, int data, int def) { // skip default if (data==def) { diff --git a/src/xml/xml_util.h b/src/xml/xml_util.h index 89231075..f1799cb4 100644 --- a/src/xml/xml_util.h +++ b/src/xml/xml_util.h @@ -163,21 +163,9 @@ class mjXUtil { static bool ReadAttrInt(tinyxml2::XMLElement* elem, const char* attr, int* data, bool required = false); - // convert string to vector - template - static std::vector String2Vector(const std::string& s); - - // write vector to string - static void Vector2String(std::string& txt, const std::vector& vec); - - // write vector to string - static void Vector2String(std::string& txt, const std::vector& vec); - // write vector to string static void Vector2String(std::string& txt, const std::vector& vec, int ncol = 0); - // write vector to string - static void Vector2String(std::string& txt, const std::vector& vec); // find subelement with given name, make sure it is unique static tinyxml2::XMLElement* FindSubElem(tinyxml2::XMLElement* elem, std::string name, @@ -212,8 +200,6 @@ class mjXUtil { template static bool ReadAttrValues(tinyxml2::XMLElement* elem, const char* attr, std::function push, int max = -1); - template static T StrToNum(char* str, char** c); - }; #endif // MUJOCO_SRC_XML_XML_UTIL_H_ diff --git a/test/user/user_util_test.cc b/test/user/user_util_test.cc index cac80c3b..02b30c08 100644 --- a/test/user/user_util_test.cc +++ b/test/user/user_util_test.cc @@ -16,93 +16,164 @@ #include "src/user/user_util.h" +#include +#include +#include + #include +#include +#include +#include "test/fixture.h" namespace mujoco { namespace { using user::FilePath; +using user::StringToVector; +using user::VectorToString; +using ::testing::ElementsAre; +using ::testing::IsNan; -TEST(UserUtilTest, PathReduce) { +using UserUtilTest = MujocoTest; + +TEST_F(UserUtilTest, PathReduce) { FilePath path = FilePath("/hello/.././world/"); EXPECT_EQ(path.Str(), "/world/"); } -TEST(UserUtilTest, PathReduce2) { +TEST_F(UserUtilTest, PathReduce2) { FilePath path = FilePath("../hello/./world/"); EXPECT_EQ(path.Str(), "../hello/world/"); } -TEST(UserUtilTest, PathReduceWin) { +TEST_F(UserUtilTest, PathReduceWin) { FilePath path = FilePath("C:\\hello\\..\\world"); EXPECT_EQ(path.Str(), "C:\\world"); } -TEST(UserUtilTest, IsAbs) { +TEST_F(UserUtilTest, IsAbs) { EXPECT_TRUE(FilePath("/hello").IsAbs()); EXPECT_TRUE(FilePath("C:\\hello").IsAbs()); EXPECT_FALSE(FilePath("hello").IsAbs()); } -TEST(UserUtilTest, Combine) { +TEST_F(UserUtilTest, Combine) { FilePath path1 = FilePath("/hello"); FilePath path2 = FilePath("world"); EXPECT_EQ((path1 + path2).Str(), "/hello/world"); } -TEST(UserUtilTest, Combine2) { +TEST_F(UserUtilTest, Combine2) { FilePath path1 = FilePath("hello/"); FilePath path2 = FilePath("world"); EXPECT_EQ((path1 + path2).Str(), "hello/world"); } -TEST(UserUtilTest, Combine3) { +TEST_F(UserUtilTest, Combine3) { FilePath path1 = FilePath("/hello"); FilePath path2 = FilePath("../world"); EXPECT_EQ((path1 + path2).Str(), "/world"); } -TEST(UserUtilTest, CombineAbs) { +TEST_F(UserUtilTest, CombineAbs) { FilePath path1 = FilePath("/hello"); FilePath path2 = FilePath("/world"); EXPECT_EQ((path1 + path2).Str(), "/world"); } -TEST(UserUtilTest, Ext) { +TEST_F(UserUtilTest, Ext) { FilePath path = FilePath("/hello/world.txt"); EXPECT_EQ(path.Ext(), ".txt"); } -TEST(UserUtilTest, ExtEmpty) { +TEST_F(UserUtilTest, ExtEmpty) { FilePath path = FilePath("/hello/world"); EXPECT_EQ(path.Ext(), ""); } -TEST(UserUtilTest, StripExt) { +TEST_F(UserUtilTest, StripExt) { FilePath path = FilePath("/hello/world.txt"); EXPECT_EQ(path.StripExt().Str(), "/hello/world"); } -TEST(UserUtilTest, StripPath) { +TEST_F(UserUtilTest, StripPath) { FilePath path = FilePath("/hello/world.txt"); EXPECT_EQ(path.StripPath().Str(), "world.txt"); } -TEST(UserUtilTest, StripPathEmpty) { +TEST_F(UserUtilTest, StripPathEmpty) { FilePath path = FilePath("world.txt"); EXPECT_EQ(path.StripPath().Str(), "world.txt"); } -TEST(UserUtilTest, StripPathWin) { +TEST_F(UserUtilTest, StripPathWin) { FilePath path = FilePath("\\world.txt"); EXPECT_EQ(path.StripPath().Str(), "world.txt"); } -TEST(UserUtilTest, StrLower) { +TEST_F(UserUtilTest, StrLower) { FilePath path = FilePath("/HELLO/worlD.txt"); EXPECT_EQ(path.StrLower(), "/hello/world.txt"); } +TEST_F(UserUtilTest, StringToVectorFloat) { + std::vector v = StringToVector(" 1.2 3.2 5.3 6 "); + EXPECT_THAT(v, ElementsAre(1.2, 3.2, 5.3, 6)); + EXPECT_EQ(errno, 0); +} + +TEST_F(UserUtilTest, StringToVectorEmpty) { + std::vector v = StringToVector(" "); + EXPECT_THAT(v, ElementsAre()); + EXPECT_EQ(errno, 0); +} + +TEST_F(UserUtilTest, StringToVectorError) { + std::vector v = StringToVector("2.1 3ABCD. /123/122/113"); + EXPECT_THAT(v, ElementsAre(2.1)); + EXPECT_EQ(errno, EINVAL); +} + +TEST_F(UserUtilTest, StringToVectorInt) { + std::vector v = StringToVector(" -1 3 5 6 "); + EXPECT_THAT(v, ElementsAre(-1, 3, 5, 6)); + EXPECT_EQ(errno, 0); +} + +TEST_F(UserUtilTest, StringToVectorString) { + auto v = StringToVector(" abc def "); + EXPECT_THAT(v, ElementsAre("abc", "def")); +} + +TEST_F(UserUtilTest, StringToVectorInvalidNumber) { + auto v = StringToVector("1 0.1.2.3"); + EXPECT_THAT(v, ElementsAre(1)); + EXPECT_EQ(errno, EINVAL); +} + +TEST_F(UserUtilTest, StringToVectorNan) { + mju_user_warning = nullptr; + auto v = StringToVector("1 2 nan 3.21"); + EXPECT_THAT(v[2], IsNan()); + EXPECT_EQ(v[3], 3.21); + EXPECT_EQ(errno, EDOM); +} + +TEST_F(UserUtilTest, StringToVectorRange) { + auto v = StringToVector("-10"); + EXPECT_EQ(errno, ERANGE); +} + +TEST_F(UserUtilTest, VectorToString) { + std::vector v = {1.2, 3.2, 5.3, 6}; + EXPECT_EQ(VectorToString(v), "1.2 3.2 5.3 6"); +} + +TEST_F(UserUtilTest, VectorToStringEmpty) { + std::vector v; + EXPECT_EQ(VectorToString(v), ""); +} + } // namespace } // namespace mujoco diff --git a/test/xml/xml_native_reader_test.cc b/test/xml/xml_native_reader_test.cc index 1da989e3..622c80fe 100644 --- a/test/xml/xml_native_reader_test.cc +++ b/test/xml/xml_native_reader_test.cc @@ -345,7 +345,7 @@ TEST_F(XMLReaderTest, InvalidArrayElement) { std::array error; mjModel* model = LoadModelFromString(xml, error.data(), error.size()); ASSERT_THAT(model, IsNull()); - EXPECT_THAT(error.data(), HasSubstr("problem reading attribute 'axisangle'")); + EXPECT_THAT(error.data(), HasSubstr("bad format in attribute 'axisangle'")); EXPECT_THAT(error.data(), HasSubstr("line 5")); } @@ -396,10 +396,26 @@ TEST_F(XMLReaderTest, InvalidNumber) { std::array error; mjModel* model = LoadModelFromString(xml, error.data(), error.size()); ASSERT_THAT(model, IsNull()); - EXPECT_THAT(error.data(), HasSubstr("problem reading attribute")); + EXPECT_THAT(error.data(), HasSubstr("bad format in attribute 'axisangle'")); EXPECT_THAT(error.data(), HasSubstr("line 5")); } +TEST_F(XMLReaderTest, InvalidNumberRange) { + static constexpr char xml[] = R"( + + + + + + )"; + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, IsNull()); + EXPECT_THAT(error.data(), + HasSubstr("number is too large in attribute 'face'")); + EXPECT_THAT(error.data(), HasSubstr("line 4")); +} + TEST_F(XMLReaderTest, InvalidNumberOfAttributes) { static constexpr char xml[] = R"( diff --git a/test/xml/xml_native_writer_test.cc b/test/xml/xml_native_writer_test.cc index d005e3e6..0d16d895 100644 --- a/test/xml/xml_native_writer_test.cc +++ b/test/xml/xml_native_writer_test.cc @@ -1238,7 +1238,7 @@ TEST_F(XMLWriterTest, SetPrecision) { EXPECT_EQ(model->geom_size[1], model_lo->geom_size[1]); EXPECT_NE(model->geom_size[2], model_lo->geom_size[2]); { - // save to XML and re-load with FullFloatPrecision + // save to XML and reload with FullFloatPrecision // expect to maintain precision FullFloatPrecision increase_precision; mjModel* model_hi = LoadModelFromString(SaveAndReadXml(model)); @@ -1276,8 +1276,10 @@ TEST_F(XMLWriterLocaleTest, IgnoresLocale) { )"; - mjModel* model = LoadModelFromString(xml); - ASSERT_THAT(model, NotNull()); + + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, NotNull()) << error.data(); std::string saved_xml = SaveAndReadXml(model); EXPECT_THAT(saved_xml, HasSubstr("0.1 1.23 2.345")); mj_deleteModel(model); diff --git a/test/xml/xml_util_test.cc b/test/xml/xml_util_test.cc deleted file mode 100644 index a0ff0b6c..00000000 --- a/test/xml/xml_util_test.cc +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2024 DeepMind Technologies Limited -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include -#include - -#include -#include -#include "src/xml/xml_util.h" -#include "test/fixture.h" - -namespace mujoco { -namespace { - -using XMLUtilTest = MujocoTest; - -using ::testing::ElementsAre; - -TEST_F(XMLUtilTest, String2VectorFloat) { - std::vector v = mjXUtil::String2Vector(" 1.2 3.2 5.3 6 "); - EXPECT_THAT(v, ElementsAre(1.2, 3.2, 5.3, 6)); -} - -TEST_F(XMLUtilTest, String2VectorEmpty) { - std::vector v = mjXUtil::String2Vector(""); - EXPECT_THAT(v, ElementsAre()); -} - -TEST_F(XMLUtilTest, String2VectorError) { - std::vector v = mjXUtil::String2Vector("ABCD. /123/122/113"); - EXPECT_THAT(v, ElementsAre()); -} - - -TEST_F(XMLUtilTest, String2VectorInt) { - std::vector v = mjXUtil::String2Vector(" -1 3 5 6"); - EXPECT_THAT(v, ElementsAre(-1, 3, 5, 6)); -} - -TEST_F(XMLUtilTest, String2VectorString) { - auto v = mjXUtil::String2Vector(" abc def "); - EXPECT_THAT(v, ElementsAre("abc", "def")); -} - -} // namespace -} // namespace mujoco