Move user_resource.c to C++.

PiperOrigin-RevId: 650592110
Change-Id: I5e043970e3aa722e3961e478334cad4f3f1c191e

src/user/CMakeLists.txt

@@ -27,7 +27,7 @@ set(MUJOCO_USER_SRCS
     user_model.h
     user_objects.cc
     user_objects.h
-    user_resource.c
+    user_resource.cc
     user_resource.h
     user_util.cc
     user_util.h

src/user/user_resource.cc

@@ -15,14 +15,16 @@
 #include "user/user_resource.h"
 
 #include <errno.h>
-#include <limits.h>
-#include <stddef.h>
-#include <stdint.h>
-#include <stdio.h>
-#include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <time.h>
+
+#include <climits>
+#include <cstddef>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+#include <ctime>
+#include <vector>
 
 #if defined (__unix__) || (defined (__APPLE__) && defined (__MACH__))
   #include <unistd.h>
@@ -37,81 +39,79 @@
 #include "engine/engine_util_errmem.h"
 #include "engine/engine_util_misc.h"
 
-// internal helper for mju_fileToMemory (closes fp automatically)
-static void* _fileToMemory(FILE* fp, const char* filename, size_t* filesize);
+namespace {
 
 // file buffer used internally for the OS filesystem
-typedef struct {
-  int is_read;      // set to nonzero if buffer was read into
-  uint8_t* buffer;  // raw bytes from file
-  size_t nbuffer;   // size of buffer in bytes
-  time_t mtime;     // last modified time
-} file_spec;
+struct FileSpec {
+  bool is_read;                 // set to nonzero if buffer was read into
+  std::vector<uint8_t> buffer;  // raw bytes from file
+  time_t mtime;                 // last modified time
+};
+
+}  // namespace
 
 // 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, char* error, size_t error_sz) {
+mjResource* mju_openResource(const char* name, char* error, size_t nerror) {
   // no error so far
   if (error) {
     error[0] = '\0';
   }
 
   mjResource* resource = (mjResource*) mju_malloc(sizeof(mjResource));
-  const mjpResourceProvider* provider = NULL;
-  if (resource == NULL) {
+  if (resource == nullptr) {
     mjERROR("could not allocate memory");
-    return NULL;
+    return nullptr;
   }
 
   // clear out resource
   memset(resource, 0, sizeof(mjResource));
 
   // copy name
-  resource->name = mju_malloc(sizeof(char) * (strlen(name) + 1));
-  if (resource->name == NULL) {
+  resource->name = (char*) mju_malloc(sizeof(char) * (strlen(name) + 1));
+  if (resource->name == nullptr) {
     mju_closeResource(resource);
     mjERROR("could not allocate memory");
-    return NULL;
+    return nullptr;
   }
   memcpy(resource->name, name, sizeof(char) * (strlen(name) + 1));
 
   // find provider based off prefix of name
-  provider = mjp_getResourceProvider(name);
-  if (provider != NULL) {
+  const mjpResourceProvider* provider = mjp_getResourceProvider(name);
+  if (provider != nullptr) {
     resource->provider = provider;
     if (provider->open(resource)) {
       return resource;
     }
 
     if (error) {
-      snprintf(error, error_sz, "could not open '%s'"
+      snprintf(error, nerror, "could not open '%s'"
                "using a resource provider matching prefix '%s'",
                name, provider->prefix);
     }
 
     mju_closeResource(resource);
-    return NULL;
+    return nullptr;
   }
 
   // lastly fallback to OS filesystem
-  resource->provider = NULL;
-  resource->data = mju_malloc(sizeof(file_spec));
-  file_spec* spec = (file_spec*) resource->data;
-  spec->is_read = 0;
-  spec->buffer = NULL;
-  spec->nbuffer = 0;
+  resource->provider = nullptr;
+  resource->data = new FileSpec;
+  FileSpec* spec = (FileSpec*) resource->data;
+  spec->is_read = false;
   struct stat file_stat;
-  errno = 0;
   if (stat(name, &file_stat) == 0) {
     memcpy(&spec->mtime, &file_stat.st_mtime, sizeof(time_t));
   } else {
     if (error) {
-      snprintf(error, error_sz, "Error opening file '%s': %s", name, strerror(errno));
+      snprintf(error, nerror, "Error opening file '%s': %s", name,
+               strerror(errno));
     }
     mju_closeResource(resource);
-    return NULL;
+    return nullptr;
   }
-  mju_encodeBase64(resource->timestamp, (uint8_t*) &spec->mtime, sizeof(time_t));
+  mju_encodeBase64(resource->timestamp, (uint8_t*) &spec->mtime,
+                   sizeof(time_t));
   return resource;
 }
 
@@ -119,7 +119,7 @@ mjResource* mju_openResource(const char* name, char* error, size_t error_sz) {
 
 // close the given resource; no-op if resource is NULL
 void mju_closeResource(mjResource* resource) {
-  if (resource == NULL) {
+  if (resource == nullptr) {
     return;
   }
 
@@ -128,10 +128,9 @@ void mju_closeResource(mjResource* resource) {
     resource->provider->close(resource);
   } else {
     // clear OS filesystem if present
-    file_spec* spec = (file_spec*) resource->data;
+    FileSpec* spec = (FileSpec*) resource->data;
     if (spec) {
-      if (spec->buffer) mju_free(spec->buffer);
-      mju_free(spec);
+      delete spec;
     }
   }
 
@@ -142,27 +141,23 @@ void mju_closeResource(mjResource* resource) {
 
 
 
-// set buffer to bytes read from the resource and return number of bytes in buffer;
-// return negative value if error
+// set buffer to bytes read from the resource and return number of bytes in
+// buffer; return negative value if error
 int mju_readResource(mjResource* resource, const void** buffer) {
-  if (resource == NULL) {
-    return 0;
-  }
-
   if (resource->provider) {
     return resource->provider->read(resource, buffer);
   }
 
   // if provider is NULL, then OS filesystem is used
-  file_spec* spec = (file_spec*) resource->data;
+  FileSpec* spec = (FileSpec*) resource->data;
 
   // only read once from file
   if (!spec->is_read) {
-    spec->buffer = mju_fileToMemory(resource->name, &(spec->nbuffer));
-    spec->is_read = 1;
+    spec->buffer = mju_fileToMemory(resource->name);
+    spec->is_read = true;
   }
-  *buffer = spec->buffer;
-  return spec->nbuffer;
+  *buffer = spec->buffer.data();
+  return spec->buffer.size();
 }
 
 
@@ -208,7 +203,7 @@ int mju_isModifiedResource(const mjResource* resource, const char* timestamp) {
 
   time_t time1, time2;
   mju_decodeBase64((uint8_t*) &time1, timestamp);
-  time2 = ((file_spec*) resource->data)->mtime;
+  time2 = ((FileSpec*) resource->data)->mtime;
   double diff = difftime(time2, time1);
   if (diff < 0) return -1;
   if (diff > 0) return  1;
@@ -236,25 +231,17 @@ int mju_dirnamelen(const char* path) {
 
 
 // read file into memory buffer (allocated here with mju_malloc)
-void* mju_fileToMemory(const char* filename, size_t* filesize) {
+std::vector<uint8_t> mju_fileToMemory(const char* filename) {
   FILE* fp = fopen(filename, "rb");
   if (!fp) {
-    return NULL;
+    return {};
   }
 
-  return _fileToMemory(fp, filename, filesize);
-}
-
-// internal helper for mju_fileToMemory (closes fp automatically)
-static void* _fileToMemory(FILE* fp, const char* filename, size_t* filesize) {
-  // open file
-  *filesize = 0;
-
   // find size
   if (fseek(fp, 0, SEEK_END) != 0) {
     fclose(fp);
     mju_warning("Failed to calculate size for '%s'", filename);
-    return NULL;
+    return {};
   }
 
   // ensure file size fits in int
@@ -262,38 +249,33 @@ static void* _fileToMemory(FILE* fp, const char* filename, size_t* filesize) {
   if (long_filesize > INT_MAX) {
     fclose(fp);
     mju_warning("File size over 2GB is not supported. File: '%s'", filename);
-    return NULL;
+    return {};
   } else if (long_filesize < 0) {
     fclose(fp);
     mju_warning("Failed to calculate size for '%s'", filename);
-    return NULL;
+    return {};
   }
-  *filesize = long_filesize;
+
+  std::vector<uint8_t> buffer(long_filesize);
 
   // go back to start of file
   if (fseek(fp, 0, SEEK_SET) != 0) {
     fclose(fp);
     mju_warning("Read error while reading '%s'", filename);
-    return NULL;
+    return {};
   }
 
   // allocate and read
-  void* buffer = mju_malloc(*filesize);
-  if (!buffer) {
-    mjERROR("could not allocate memory");
-  }
-  size_t bytes_read = fread(buffer, 1, *filesize, fp);
+  std::size_t bytes_read = fread(buffer.data(), 1, buffer.size(), fp);
 
   // check that read data matches file size
-  if (bytes_read != *filesize) {  // SHOULD NOT OCCUR
+  if (bytes_read != buffer.size()) {  // SHOULD NOT OCCUR
     if (ferror(fp)) {
       fclose(fp);
-      mju_free(buffer);
-      *filesize = 0;
       mju_warning("Read error while reading '%s'", filename);
-      return NULL;
+      return {};
     } else if (feof(fp)) {
-      *filesize = bytes_read;
+      buffer.resize(bytes_read);
     }
   }
 

src/user/user_resource.h

@@ -17,7 +17,9 @@
 #ifndef MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_
 #define MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_
 
-#include <stddef.h>
+#include <cstddef>
+#include <cstdint>
+#include <vector>
 
 #include <mujoco/mjexport.h>
 #include <mujoco/mujoco.h>
@@ -28,7 +30,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, char* error, size_t error_sz);
+MJAPI mjResource* mju_openResource(const char* name, char* error, std::size_t nerror);
 
 // close the given resource; no-op if resource is NULL
 MJAPI void mju_closeResource(mjResource* resource);
@@ -46,13 +48,13 @@ MJAPI void mju_getResourceDir(mjResource* resource, const char** dir, int* ndir)
 MJAPI int mju_isModifiedResource(const mjResource* resource, const char* timestamp);
 
 // get the length of the dirname portion of a given path
-int mju_dirnamelen(const char* path);
-
-// read file into memory buffer (allocated here with mju_malloc)
-void* mju_fileToMemory(const char* filename, size_t* filesize);
+MJAPI int mju_dirnamelen(const char* path);
 
 #ifdef __cplusplus
 }
 #endif
 
+// read file into memory buffer (allocated here with mju_malloc)
+std::vector<uint8_t> mju_fileToMemory(const char* filename);
+
 #endif  // MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_

src/user/user_vfs.cc

@@ -19,10 +19,10 @@
 #include <cstddef>
 #include <cstring>
 #include <cstdint>
-#include <functional>
-#include <memory>
 #include <string>
 #include <unordered_map>
+#include <utility>
+#include <vector>
 
 #include "engine/engine_util_errmem.h"
 #include "engine/engine_util_misc.h"
@@ -34,7 +34,7 @@ namespace {
 // internal struct for VFS files
 struct VFSFile {
   std::string filename;
-  std::unique_ptr<void, std::function<void(void*)>> filedata;
+  std::vector<uint8_t> filedata;
   std::size_t filesize;
   uint64_t filestamp;
 };
@@ -46,9 +46,9 @@ class VFS {
   bool HasFile(const std::string& filename) const;
 
   // returns inserted mjuuVFSFile if the file was added successfully. This class
-  // assumes ownership of the buffer and will free it when the VFS is deleted.
-  VFSFile* AddFile(const std::string& filename, void* buffer,
-                   std::size_t nbuffer, uint64_t filestamp);
+  // assumes ownership of the buffer.
+  VFSFile* AddFile(const std::string& filename, std::vector<uint8_t>&& buffer,
+                   uint64_t filestamp);
 
   // returns the internal file struct for the given filename
   const VFSFile* GetFile(const std::string& filename) const;
@@ -76,13 +76,12 @@ std::string StripPath(const char* name) {
 }
 
 // copies data into a buffer and produces a hash of the data
-uint64_t vfs_memcpy(void* dest, const void* src, size_t n) {
+uint64_t vfs_memcpy(std::vector<uint8_t>& dest, const void* src, size_t n) {
   uint64_t hash = 0xcbf29ce484222325;  // magic number
   uint64_t prime = 0x100000001b3;      // magic prime
   const uint8_t* bytes = (uint8_t*) src;
-  uint8_t* buffer = (uint8_t*) dest;
   for (size_t i = 0; i < n; i++) {
-    buffer[i] = bytes[i];
+    dest.push_back(bytes[i]);
 
     // do FNV-1 hash
     hash |= bytes[i];
@@ -92,11 +91,12 @@ uint64_t vfs_memcpy(void* dest, const void* src, size_t n) {
 }
 
 // VFS hash function implemented using the FNV-1 hash
-uint64_t vfs_hash(const void* buffer, size_t n) {
+uint64_t vfs_hash(const std::vector<uint8_t>& buffer) {
   uint64_t hash = 0xcbf29ce484222325;  // magic number
   uint64_t prime = 0x100000001b3;      // magic prime
-  const uint8_t* bytes = (uint8_t*) buffer;
-  for (size_t i = 0; i < n; i++) {
+  const uint8_t* bytes = (uint8_t*) buffer.data();
+  std::size_t n = buffer.size();
+  for (std::size_t i = 0; i < n; i++) {
     hash |= bytes[i];
     hash *= prime;
   }
@@ -107,16 +107,14 @@ bool VFS::HasFile(const std::string& filename) const {
   return files_.find(filename) != files_.end();
 }
 
-VFSFile* VFS::AddFile(const std::string& filename, void* buffer,
-                      std::size_t nbuffer, uint64_t filestamp) {
+VFSFile* VFS::AddFile(const std::string& filename, std::vector<uint8_t>&& buffer,
+                      uint64_t filestamp) {
   auto [it, inserted] = files_.insert({filename, VFSFile()});
   if (!inserted) {
     return nullptr;  // repeated name
   }
   it->second.filename = filename;
-  it->second.filedata = std::unique_ptr<void, void(*)(void*)>(
-      buffer, [](void* b) { mju_free(b); });  // corresponding to mju_malloc
-  it->second.filesize = nbuffer;
+  it->second.filedata = buffer;
   it->second.filestamp = filestamp;
   return &(it->second);
 }
@@ -173,8 +171,8 @@ int Read(mjResource* resource, const void** buffer) {
     return -1;
   }
 
-  *buffer = file->filedata.get();
-  return file->filesize;
+  *buffer = file->filedata.data();
+  return file->filedata.size();
 }
 
 // close callback for the VFS resource provider
@@ -234,14 +232,12 @@ int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) {
   }
 
   // allocate and read
-  size_t nbuffer = 0;
-  void* buffer = mju_fileToMemory(fullname.c_str(), &nbuffer);
-  if (buffer == nullptr) {
+  std::vector<uint8_t> buffer = mju_fileToMemory(fullname.c_str());
+  if (buffer.empty()) {
     return -1;
   }
 
-  if (!cvfs->AddFile(newname, buffer, nbuffer, vfs_hash(buffer, nbuffer))) {
-    mju_free(buffer);
+  if (!cvfs->AddFile(newname, std::move(buffer), vfs_hash(buffer))) {
     return 2;  // AddFile failed, SHOULD NOT OCCUR
   }
   return 0;
@@ -250,19 +246,14 @@ int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename) {
 // add file from buffer into VFS
 int mj_addBufferVFS(mjVFS* vfs, const char* name, const void* buffer,
                     int nbuffer) {
+  std::vector<uint8_t> inbuffer;
   VFS* cvfs = GetVFSImpl(vfs);
-
-  // allocate and clear
-  void* inbuffer = mju_malloc(nbuffer);
-  if (buffer == nullptr) {
-    mjERROR("could not allocate memory");
-  }
   VFSFile* file;
-  if (!(file = cvfs->AddFile(StripPath(name), inbuffer, nbuffer, 0))) {
-    mju_free(inbuffer);
+  if (!(file = cvfs->AddFile(StripPath(name), std::move(inbuffer), 0))) {
     return 2;  // AddFile failed, repeated name
   }
-  file->filestamp = vfs_memcpy(inbuffer, buffer, nbuffer);
+  file->filedata.reserve(nbuffer);
+  file->filestamp = vfs_memcpy(file->filedata, buffer, nbuffer);
   return 0;
 }
 

src/user/user_vfs.h

@@ -33,9 +33,6 @@ MJAPI void mj_defaultVFS(mjVFS* vfs);
 // add file to VFS, return 0: success, 2: repeated name, -1: not found on disk
 MJAPI int mj_addFileVFS(mjVFS* vfs, const char* directory, const char* filename);
 
-// deprecated: use mj_addBufferVFS
-MJAPI int mj_makeEmptyFileVFS(mjVFS* vfs, const char* filename, int filesize);
-
 // add file from buffer into VFS, return 0: success, 2: repeated name, -1: failed to load
 MJAPI int mj_addBufferVFS(mjVFS* vfs, const char* filename, const void* buffer, int nbuffer);
 

test/user/user_vfs_test.cc

@@ -180,7 +180,7 @@ TEST_F(UserVfsTest, AddBuffer) {
                   buffer.size());
   std::array<char, 1024> error;
   mjModel* model = mj_loadXML("model", &vfs, error.data(), error.size());
-  EXPECT_THAT(model, NotNull());
+  ASSERT_THAT(model, NotNull()) << error.data();
   mj_deleteModel(model);
   mj_deleteVFS(&vfs);
 }