Convert mju_openResource to write warnings to a given error string.

PiperOrigin-RevId: 606907392
Change-Id: Ic70e980aa9ba2bdbe1787773f51749b34f74afed

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_openVfsResource(filename, vfs))) {
-    if ((r = mju_openResource(filename)) == NULL) {
+    char error[1024];
+    if (!(r = mju_openResource(filename, error, 1024))) {
+       mju_warning("%s", error);
       return NULL;
     }
   }

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' "
+    if (error) {
-                "using a resource provider matching prefix '%s'",
+      snprintf(error, error_sz, "could not open '%s'"
-                name, provider->prefix);
+               "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;
 }
 

src/engine/engine_resource.h

@@ -18,7 +18,7 @@
 #include <stddef.h>
 
 #include <mujoco/mjexport.h>
-#include "engine/engine_plugin.h"
+#include <mujoco/mujoco.h>
 
 #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);

src/user/user_objects.cc

@@ -15,12 +15,12 @@
 #include "user/user_objects.h"
 
 #include <algorithm>
+#include <array>
 #include <cmath>
 #include <cstddef>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
-#include <iostream>
 #include <optional>
 #include <sstream>
 #include <string>
@@ -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<char, 1024> error;
     // not in vfs try a provider or fallback to OS filesystem
-    if ((r = mju_openResource(filename.c_str())) == nullptr) {
+    r = mju_openResource(filename.c_str(), error.data(), error.size());
-      throw mjCError(nullptr, "resource not found via provider or OS filesystem: '%s'", cname);
+    if (!r) {
+      throw mjCError(nullptr, "%s", error.data());
     }
   }
   return r;

src/xml/xml.cc

@@ -20,6 +20,7 @@
 #include <xlocale.h>
 #endif
 
+#include <array>
 #include <cstdio>
 #include <string>
 #include <unordered_set>
@@ -140,11 +141,13 @@ static void mjIncludeXML(XMLElement* elem, string dir, const mjVFS* vfs,
   }
 
   // get data source
-  mjResource *resource = nullptr;
+  mjResource *resource = mju_openVfsResource(filename.c_str(), vfs);
-  if ((resource = mju_openVfsResource(filename.c_str(), vfs)) == nullptr) {
+  if (!resource) {
     // load from provider or OS filesystem
-    if ((resource = mju_openResource(filename.c_str())) == nullptr) {
+    std::array<char, 1024> error;
-      throw mjXError(elem, "Could not open file '%s'", filename.c_str());
+    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) {
+    std::array<char, 1024> rerror;
-      if (error) {
+    resource = mju_openResource(filename, rerror.data(), rerror.size());
-        std::snprintf(error, error_sz, "mjParseXML: could not open file '%s'", filename);
+    if (!resource) {
-      }
+      std::snprintf(error, error_sz, "mjParseXML: %s", rerror.data());
       return nullptr;
     }
   }

test/engine/engine_resource_test.cc

@@ -14,6 +14,7 @@
 
 // Tests for engine/engine_resource.c
 
+#include <array>
 #include <cstring>
 
 #include <gmock/gmock.h>
@@ -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);
 
-
+  static std::array<char, 1024> error;
-  // install warning handler
-  static char warning[1024];
-  warning[0] = '\0';
-  mju_user_warning = [](const char* msg) {
-    util::strcpy_arr(warning, msg);
-  };
 
   // 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());
 }