diff --git a/src/engine/engine_io.c b/src/engine/engine_io.c index 1b1975e2..bf99d38f 100644 --- a/src/engine/engine_io.c +++ b/src/engine/engine_io.c @@ -722,8 +722,10 @@ mjModel* mj_loadModel(const char* filename, const mjVFS* vfs) { mjResource* r = NULL; // first try vfs, otherwise try a provider or OS filesystem - if ((r = mju_openVfsResource(filename, vfs)) == NULL) { - if ((r = mju_openResource(filename)) == NULL) { + if (!(r = mju_openVfsResource(filename, vfs))) { + char error[1024]; + if (!(r = mju_openResource(filename, error, 1024))) { + mju_warning("%s", error); return NULL; } } diff --git a/src/engine/engine_resource.c b/src/engine/engine_resource.c index 4718df50..df94efd2 100644 --- a/src/engine/engine_resource.c +++ b/src/engine/engine_resource.c @@ -44,7 +44,12 @@ typedef struct { // open the given resource; if the name doesn't have a prefix matching with a // resource provider, then the OS filesystem is used -mjResource* mju_openResource(const char* name) { +mjResource* mju_openResource(const char* name, char* error, size_t error_sz) { + // no error so far + if (error) { + error[0] = '\0'; + } + mjResource* resource = (mjResource*) mju_malloc(sizeof(mjResource)); const mjpResourceProvider* provider = NULL; if (resource == NULL) { @@ -72,9 +77,12 @@ mjResource* mju_openResource(const char* name) { return resource; } - mju_warning("mju_openResource: could not open resource '%s' " - "using a resource provider matching prefix '%s'", - name, provider->prefix); + if (error) { + snprintf(error, error_sz, "could not open '%s'" + "using a resource provider matching prefix '%s'", + name, provider->prefix); + } + mju_closeResource(resource); return NULL; } @@ -85,7 +93,10 @@ mjResource* mju_openResource(const char* name) { file_buffer* fb = (file_buffer*) resource->data; fb->buffer = mju_fileToMemory(name, &(fb->nbuffer)); if (fb->buffer == NULL) { - mju_warning("mju_openResource: unknown file '%s'", name); + if (error) { + snprintf(error, error_sz, + "resource not found via provider or OS filesystem: '%s'", name); + } mju_closeResource(resource); return NULL; } @@ -95,7 +106,6 @@ mjResource* mju_openResource(const char* name) { } else { memset(&fb->mtime, 0, sizeof(time_t)); } - return resource; } diff --git a/src/engine/engine_resource.h b/src/engine/engine_resource.h index b45b3a82..96f35015 100644 --- a/src/engine/engine_resource.h +++ b/src/engine/engine_resource.h @@ -18,7 +18,7 @@ #include #include -#include "engine/engine_plugin.h" +#include #ifdef __cplusplus extern "C" { @@ -26,7 +26,7 @@ extern "C" { // open the given resource; if the name doesn't have a prefix matching with a // resource provider, then the OS filesystem is used -MJAPI mjResource* mju_openResource(const char* name); +MJAPI mjResource* mju_openResource(const char* name, char* error, size_t error_sz); // close the given resource; no-op if resource is NULL MJAPI void mju_closeResource(mjResource* resource); diff --git a/src/user/user_objects.cc b/src/user/user_objects.cc index a17e4fbd..9c61e483 100644 --- a/src/user/user_objects.cc +++ b/src/user/user_objects.cc @@ -15,12 +15,12 @@ #include "user/user_objects.h" #include +#include #include #include #include #include #include -#include #include #include #include @@ -515,14 +515,15 @@ mjCBase::mjCBase() { // load resource if found (fallback to OS filesystem) mjResource* mjCBase::LoadResource(string filename, const mjVFS* vfs) { - mjResource* r = nullptr; - const char* cname = filename.c_str(); - // try reading from provided VFS - if ((r = mju_openVfsResource(cname, vfs)) == nullptr) { + mjResource* r = mju_openVfsResource(filename.c_str(), vfs); + + if (!r) { + std::array error; // not in vfs try a provider or fallback to OS filesystem - if ((r = mju_openResource(filename.c_str())) == nullptr) { - throw mjCError(nullptr, "resource not found via provider or OS filesystem: '%s'", cname); + r = mju_openResource(filename.c_str(), error.data(), error.size()); + if (!r) { + throw mjCError(nullptr, "%s", error.data()); } } return r; diff --git a/src/xml/xml.cc b/src/xml/xml.cc index 706a0e7c..be768c20 100644 --- a/src/xml/xml.cc +++ b/src/xml/xml.cc @@ -20,6 +20,7 @@ #include #endif +#include #include #include #include @@ -140,11 +141,13 @@ static void mjIncludeXML(XMLElement* elem, string dir, const mjVFS* vfs, } // get data source - mjResource *resource = nullptr; - if ((resource = mju_openVfsResource(filename.c_str(), vfs)) == nullptr) { + mjResource *resource = mju_openVfsResource(filename.c_str(), vfs); + if (!resource) { // load from provider or OS filesystem - if ((resource = mju_openResource(filename.c_str())) == nullptr) { - throw mjXError(elem, "Could not open file '%s'", filename.c_str()); + std::array error; + resource = mju_openResource(filename.c_str(), error.data(), error.size()); + if (!resource) { + throw mjXError(elem, "%s", error.data()); } } @@ -232,14 +235,15 @@ mjCModel* mjParseXML(const char* filename, const mjVFS* vfs, char* error, int er } // get data source - mjResource* resource = nullptr; const char* xmlstring = nullptr; - if ((resource = mju_openVfsResource(filename, vfs)) == nullptr) { + mjResource* resource = mju_openVfsResource(filename, vfs); + + if (!resource) { // load from provider or fallback to OS filesystem - if ((resource = mju_openResource(filename)) == nullptr) { - if (error) { - std::snprintf(error, error_sz, "mjParseXML: could not open file '%s'", filename); - } + std::array rerror; + resource = mju_openResource(filename, rerror.data(), rerror.size()); + if (!resource) { + std::snprintf(error, error_sz, "mjParseXML: %s", rerror.data()); return nullptr; } } diff --git a/test/engine/engine_resource_test.cc b/test/engine/engine_resource_test.cc index c3287375..07390b63 100644 --- a/test/engine/engine_resource_test.cc +++ b/test/engine/engine_resource_test.cc @@ -14,6 +14,7 @@ // Tests for engine/engine_resource.c +#include #include #include @@ -228,7 +229,7 @@ TEST_F(ResourceTest, GeneralTest) { EXPECT_GT(i, 0); // open resource - mjResource* resource = mju_openResource("str:file"); + mjResource* resource = mju_openResource("str:file", nullptr, 0); ASSERT_THAT(resource, NotNull()); const char* buffer = NULL; @@ -239,7 +240,7 @@ TEST_F(ResourceTest, GeneralTest) { mju_closeResource(resource); } -TEST_F(ResourceTest, GeneralTestFailure) { +TEST_F(ResourceTest, GeneralFailureTest) { mjpResourceProvider provider = { "str", open_str, read_str, close_str }; @@ -248,19 +249,14 @@ TEST_F(ResourceTest, GeneralTestFailure) { int i = mjp_registerResourceProvider(&provider); EXPECT_GT(i, 0); - - // install warning handler - static char warning[1024]; - warning[0] = '\0'; - mju_user_warning = [](const char* msg) { - util::strcpy_arr(warning, msg); - }; + static std::array error; // open resource - mjResource* resource = mju_openResource("str:notfound"); + mjResource* resource = mju_openResource("str:notfound", + error.data(), error.size()); ASSERT_THAT(resource, IsNull()); - EXPECT_THAT(warning, HasSubstr("could not open")); + EXPECT_THAT(error.data(), HasSubstr("could not open")); } TEST_F(ResourceTest, NameWithValidPrefix) { @@ -281,7 +277,7 @@ TEST_F(ResourceTest, NameWithValidPrefix) { }; // open resource - mjResource* resource = mju_openResource("nop:found"); + mjResource* resource = mju_openResource("nop:found", nullptr, 0); ASSERT_THAT(resource, NotNull()); mju_closeResource(resource); } @@ -304,7 +300,7 @@ TEST_F(ResourceTest, NameWithUpperCasePrefix) { }; // open resource - mjResource* resource = mju_openResource("NOP:found"); + mjResource* resource = mju_openResource("NOP:found", nullptr, 0); ASSERT_THAT(resource, NotNull()); mju_closeResource(resource); } @@ -327,7 +323,7 @@ TEST_F(ResourceTest, NameWithInvalidPrefix) { }; // open resource - mjResource* resource = mju_openResource("nopfound"); + mjResource* resource = mju_openResource("nopfound", nullptr, 0); ASSERT_THAT(resource, IsNull()); }