From e310c23267dead8a1099916440e18d33233ea246 Mon Sep 17 00:00:00 2001 From: Alessio Quaglino Date: Tue, 26 Nov 2024 02:31:53 -0800 Subject: [PATCH] Fix return value of mj_saveXML and mj_saveXMLString, fixes #2247 PiperOrigin-RevId: 700269761 Change-Id: Ifb7e9c66c70eac7d681cfcf106712d47291cf9e2 --- doc/APIreference/functions.rst | 5 +++-- doc/APIreference/functions_override.rst | 5 +++-- include/mujoco/mujoco.h | 3 ++- introspect/functions.py | 2 +- src/xml/xml_api.cc | 11 ++++++----- src/xml/xml_api.h | 2 +- test/xml/xml_api_test.cc | 3 +++ 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/doc/APIreference/functions.rst b/doc/APIreference/functions.rst index 7a2b96cf..c80c9628 100644 --- a/doc/APIreference/functions.rst +++ b/doc/APIreference/functions.rst @@ -94,7 +94,8 @@ Free last XML model if loaded. Called internally at each load. .. mujoco-include:: mj_saveXMLString -Save spec to XML string, return 1 on success, 0 otherwise. XML saving requires that the spec first be compiled. +Save spec to XML string, return 0 on success, -1 on failure. If the length of the output buffer is too small, returns +the required size. XML saving requires that the spec first be compiled. .. _mj_saveXML: @@ -103,7 +104,7 @@ Save spec to XML string, return 1 on success, 0 otherwise. XML saving requires t .. mujoco-include:: mj_saveXML -Save spec to XML file, return 1 on success, 0 otherwise. XML saving requires that the spec first be compiled. +Save spec to XML file, return 0 on success, -1 otherwise. XML saving requires that the spec first be compiled. .. _Mainsimulation: diff --git a/doc/APIreference/functions_override.rst b/doc/APIreference/functions_override.rst index 447a0a05..d34fa3f9 100644 --- a/doc/APIreference/functions_override.rst +++ b/doc/APIreference/functions_override.rst @@ -49,11 +49,12 @@ instances will be deleted; as in :ref:`mj_compile`, the compilation error can be .. _mj_saveXMLString: -Save spec to XML string, return 1 on success, 0 otherwise. XML saving requires that the spec first be compiled. +Save spec to XML string, return 0 on success, -1 on failure. If the length of the output buffer is too small, returns +the required size. XML saving requires that the spec first be compiled. .. _mj_saveXML: -Save spec to XML file, return 1 on success, 0 otherwise. XML saving requires that the spec first be compiled. +Save spec to XML file, return 0 on success, -1 otherwise. XML saving requires that the spec first be compiled. .. _Mainsimulation: diff --git a/include/mujoco/mujoco.h b/include/mujoco/mujoco.h index 0c09f566..2e2cab59 100644 --- a/include/mujoco/mujoco.h +++ b/include/mujoco/mujoco.h @@ -117,7 +117,8 @@ MJAPI int mj_saveLastXML(const char* filename, const mjModel* m, char* error, in // Free last XML model if loaded. Called internally at each load. MJAPI void mj_freeLastXML(void); -// Save spec to XML string, return 1 on success, 0 otherwise. +// Save spec to XML string, return 0 on success, -1 on failure. +// If length of the output buffer is too small, returns the required size. MJAPI int mj_saveXMLString(const mjSpec* s, char* xml, int xml_sz, char* error, int error_sz); // Save spec to XML file, return 1 on success, 0 otherwise. diff --git a/introspect/functions.py b/introspect/functions.py index 6a936263..4e1baaad 100644 --- a/introspect/functions.py +++ b/introspect/functions.py @@ -349,7 +349,7 @@ FUNCTIONS: Mapping[str, FunctionDecl] = dict([ type=ValueType(name='int'), ), ), - doc='Save spec to XML string, return 1 on success, 0 otherwise.', + doc='Save spec to XML string, return 0 on success, -1 on failure. If length of the output buffer is too small, returns the required size.', # pylint: disable=line-too-long )), ('mj_saveXML', FunctionDecl( diff --git a/src/xml/xml_api.cc b/src/xml/xml_api.cc index 8fadc4cb..627e2699 100644 --- a/src/xml/xml_api.cc +++ b/src/xml/xml_api.cc @@ -226,23 +226,24 @@ mjSpec* mj_parseXMLString(const char* xml, const mjVFS* vfs, char* error, int er -// save spec to XML file, return 1 on success, 0 otherwise +// save spec to XML file, return 0 on success, -1 otherwise int mj_saveXML(const mjSpec* s, const char* filename, char* error, int error_sz) { std::string result = WriteXML(NULL, s, error, error_sz); if (result.empty()) { - return 0; + return -1; } std::ofstream file; file.open(filename); file << result; file.close(); - return 1; + return 0; } -// save spec to string, return 1 on success, 0 otherwise +// save spec to XML string, return 0 on success, -1 on failure +// if length of the output buffer is too small, returns the required size int mj_saveXMLString(const mjSpec* s, char* xml, int xml_sz, char* error, int error_sz) { std::string result = WriteXML(NULL, s, error, error_sz); if (result.size() >= xml_sz) { @@ -252,7 +253,7 @@ int mj_saveXMLString(const mjSpec* s, char* xml, int xml_sz, char* error, int er return result.size(); } if (result.empty()) { - return 0; + return -1; } result.copy(xml, xml_sz); diff --git a/src/xml/xml_api.h b/src/xml/xml_api.h index fe4d0f18..8bb467c4 100644 --- a/src/xml/xml_api.h +++ b/src/xml/xml_api.h @@ -48,7 +48,7 @@ MJAPI mjModel* mj_loadModel(const char* filename, const mjVFS* vfs); MJAPI mjSpec* mj_parseXML(const char* filename, const mjVFS* vfs, char* error, int error_sz); MJAPI mjSpec* mj_parseXMLString(const char* xml, const mjVFS* vfs, char* error, int error_sz); -// Save spec to XML file and/or string, return 1 on success, 0 otherwise. +// Save spec to XML file and/or string, return 0 on success, -1 otherwise. MJAPI int mj_saveXML(const mjSpec* s, const char* filename, char* error, int error_sz); MJAPI int mj_saveXMLString(const mjSpec* s, char* xml, int xml_sz, char* error, int error_sz); diff --git a/test/xml/xml_api_test.cc b/test/xml/xml_api_test.cc index 9b5b241c..dea098c6 100644 --- a/test/xml/xml_api_test.cc +++ b/test/xml/xml_api_test.cc @@ -140,6 +140,9 @@ TEST_F(MujocoTest, SaveXml) { EXPECT_THAT(model, NotNull()) << "Failed to compile model: " << error.data(); std::array out; + EXPECT_THAT(mj_saveXMLString(NULL, out.data(), out.size(), error.data(), + error.size()), -1); + EXPECT_STREQ(error.data(), "Cannot write empty model"); EXPECT_THAT(mj_saveXMLString(spec, out.data(), out.size(), error.data(), error.size()), 0) << error.data();