diff --git a/src/user/CMakeLists.txt b/src/user/CMakeLists.txt index 0f507be2..6bd7399a 100644 --- a/src/user/CMakeLists.txt +++ b/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 diff --git a/src/user/user_resource.c b/src/user/user_resource.cc similarity index 70% rename from src/user/user_resource.c rename to src/user/user_resource.cc index 2a746079..ebe21238 100644 --- a/src/user/user_resource.c +++ b/src/user/user_resource.cc @@ -15,14 +15,16 @@ #include "user/user_resource.h" #include -#include -#include -#include -#include -#include #include #include -#include + +#include +#include +#include +#include +#include +#include +#include #if defined (__unix__) || (defined (__APPLE__) && defined (__MACH__)) #include @@ -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 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 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 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); } } diff --git a/src/user/user_resource.h b/src/user/user_resource.h index 0a7068e0..3a564091 100644 --- a/src/user/user_resource.h +++ b/src/user/user_resource.h @@ -17,7 +17,9 @@ #ifndef MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_ #define MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_ -#include +#include +#include +#include #include #include @@ -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 mju_fileToMemory(const char* filename); + #endif // MUJOCO_SRC_ENGINE_ENGINE_RESOURCE_H_ diff --git a/src/user/user_vfs.cc b/src/user/user_vfs.cc index 417a7597..8d6188a8 100644 --- a/src/user/user_vfs.cc +++ b/src/user/user_vfs.cc @@ -19,10 +19,10 @@ #include #include #include -#include -#include #include #include +#include +#include #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> filedata; + std::vector 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&& 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& 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& 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&& 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( - 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 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 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; } diff --git a/src/user/user_vfs.h b/src/user/user_vfs.h index 202136fd..c133ef66 100644 --- a/src/user/user_vfs.h +++ b/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); diff --git a/test/user/user_vfs_test.cc b/test/user/user_vfs_test.cc index 48468477..b5931ed6 100644 --- a/test/user/user_vfs_test.cc +++ b/test/user/user_vfs_test.cc @@ -180,7 +180,7 @@ TEST_F(UserVfsTest, AddBuffer) { buffer.size()); std::array 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); }