Refactor LLFileSystem for and fix an old issue in LLFile (#2332)

7 files changed, 39 insertions, 135 deletions

diff --git a/indra/llcommon/llfile.cpp b/indra/llcommon/llfile.cpp
index 9045324bf2..ed94ef21ef 100644
--- a/indra/llcommon/llfile.cpp
+++ b/indra/llcommon/llfile.cpp
@@ -293,7 +293,7 @@ int LLFile::rename(const std::string& filename, const std::string& newname, int
     return warnif(STRINGIZE("rename to '" << newname << "' from"), filename, rc, supress_error);
 }
 
-bool LLFile::copy(const std::string from, const std::string to)
+bool LLFile::copy(const std::string& from, const std::string& to)
 {
     bool copied = false;
     LLFILE* in = LLFile::fopen(from, "rb");     /* Flawfinder: ignore */
@@ -424,7 +424,7 @@ LLFILE *    LLFile::_Fiopen(const std::string& filename,
 
     if (valid[n] == 0)
         return (0); // no valid mode
-    else if (norepflag && mode & (ios_base::out || ios_base::app)
+    else if (norepflag && mode & (ios_base::out | ios_base::app)
         && (fp = LLFile::fopen(filename, "r")) != 0)    /* Flawfinder: ignore */
         {   // file must not exist, close and fail
         fclose(fp);
diff --git a/indra/llcommon/llfile.h b/indra/llcommon/llfile.h
index 74110343fc..1661cbeb55 100644
--- a/indra/llcommon/llfile.h
+++ b/indra/llcommon/llfile.h
@@ -77,7 +77,7 @@ public:
     static  int     rmdir(const std::string& filename);
     static  int     remove(const std::string& filename, int supress_error = 0);
     static  int     rename(const std::string& filename,const std::string& newname, int supress_error = 0);
-    static  bool    copy(const std::string from, const std::string to);
+    static  bool    copy(const std::string& from, const std::string& to);
 
     static  int     stat(const std::string& filename,llstat*    file_status);
     static  bool    isdir(const std::string&    filename);
@@ -160,7 +160,7 @@ private:
  *  Does The Right Thing when passed a non-ASCII pathname. Sadly, that isn't
  *  true of Microsoft's std::ifstream.
  */
-class LL_COMMON_API llifstream  :   public  std::ifstream
+class LL_COMMON_API llifstream : public std::ifstream
 {
     // input stream associated with a C stream
   public:
@@ -205,7 +205,7 @@ class LL_COMMON_API llifstream  :   public  std::ifstream
  *  Right Thing when passed a non-ASCII pathname. Sadly, that isn't true of
  *  Microsoft's std::ofstream.
 */
-class LL_COMMON_API llofstream  :   public  std::ofstream
+class LL_COMMON_API llofstream : public std::ofstream
 {
   public:
     // Constructors:
@@ -241,7 +241,7 @@ class LL_COMMON_API llofstream  :   public  std::ofstream
 
 
 /**
- * @breif filesize helpers.
+ * @brief filesize helpers.
  *
  * The file size helpers are not considered particularly efficient,
  * and should only be used for config files and the like -- not in a
diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp
index e780387f4e..49904911a9 100644
--- a/indra/llfilesystem/lldiskcache.cpp
+++ b/indra/llfilesystem/lldiskcache.cpp
@@ -51,7 +51,7 @@ static const std::string CACHE_FILENAME_PREFIX("sl_cache");
 
 std::string LLDiskCache::sCacheDir;
 
-LLDiskCache::LLDiskCache(const std::string cache_dir,
+LLDiskCache::LLDiskCache(const std::string& cache_dir,
                          const uintmax_t max_size_bytes,
                          const bool enable_cache_debug_info) :
     mMaxSizeBytes(max_size_bytes),
@@ -196,59 +196,9 @@ void LLDiskCache::purge()
     }
 }
 
-const std::string LLDiskCache::assetTypeToString(LLAssetType::EType at)
+const std::string LLDiskCache::metaDataToFilepath(const LLUUID& id, LLAssetType::EType at)
 {
-    /**
-     * Make use of the handy C++17  feature that allows
-     * for inline initialization of an std::map<>
-     */
-    typedef std::map<LLAssetType::EType, std::string> asset_type_to_name_t;
-    asset_type_to_name_t asset_type_to_name =
-    {
-        { LLAssetType::AT_TEXTURE, "TEXTURE" },
-        { LLAssetType::AT_SOUND, "SOUND" },
-        { LLAssetType::AT_CALLINGCARD, "CALLINGCARD" },
-        { LLAssetType::AT_LANDMARK, "LANDMARK" },
-        { LLAssetType::AT_SCRIPT, "SCRIPT" },
-        { LLAssetType::AT_CLOTHING, "CLOTHING" },
-        { LLAssetType::AT_OBJECT, "OBJECT" },
-        { LLAssetType::AT_NOTECARD, "NOTECARD" },
-        { LLAssetType::AT_CATEGORY, "CATEGORY" },
-        { LLAssetType::AT_LSL_TEXT, "LSL_TEXT" },
-        { LLAssetType::AT_LSL_BYTECODE, "LSL_BYTECODE" },
-        { LLAssetType::AT_TEXTURE_TGA, "TEXTURE_TGA" },
-        { LLAssetType::AT_BODYPART, "BODYPART" },
-        { LLAssetType::AT_SOUND_WAV, "SOUND_WAV" },
-        { LLAssetType::AT_IMAGE_TGA, "IMAGE_TGA" },
-        { LLAssetType::AT_IMAGE_JPEG, "IMAGE_JPEG" },
-        { LLAssetType::AT_ANIMATION, "ANIMATION" },
-        { LLAssetType::AT_GESTURE, "GESTURE" },
-        { LLAssetType::AT_SIMSTATE, "SIMSTATE" },
-        { LLAssetType::AT_LINK, "LINK" },
-        { LLAssetType::AT_LINK_FOLDER, "LINK_FOLDER" },
-        { LLAssetType::AT_MARKETPLACE_FOLDER, "MARKETPLACE_FOLDER" },
-        { LLAssetType::AT_WIDGET, "WIDGET" },
-        { LLAssetType::AT_PERSON, "PERSON" },
-        { LLAssetType::AT_MESH, "MESH" },
-        { LLAssetType::AT_SETTINGS, "SETTINGS" },
-        { LLAssetType::AT_MATERIAL, "MATERIAL" },
-        { LLAssetType::AT_GLTF, "GLTF" },
-        { LLAssetType::AT_GLTF_BIN, "GLTF_BIN" },
-        { LLAssetType::AT_UNKNOWN, "UNKNOWN" }
-    };
-
-    asset_type_to_name_t::iterator iter = asset_type_to_name.find(at);
-    if (iter != asset_type_to_name.end())
-    {
-        return iter->second;
-    }
-
-    return std::string("UNKNOWN");
-}
-
-const std::string LLDiskCache::metaDataToFilepath(const std::string& id, LLAssetType::EType at)
-{
-    return llformat("%s%s%s_%s_0.asset", sCacheDir.c_str(), gDirUtilp->getDirDelimiter().c_str(), CACHE_FILENAME_PREFIX.c_str(), id.c_str());
+    return llformat("%s%s%s_%s_0.asset", sCacheDir.c_str(), gDirUtilp->getDirDelimiter().c_str(), CACHE_FILENAME_PREFIX.c_str(), id.asString().c_str());
 }
 
 const std::string LLDiskCache::getCacheInfo()
@@ -335,7 +285,7 @@ void LLDiskCache::removeOldVFSFiles()
     }
 }
 
-uintmax_t LLDiskCache::dirFileSize(const std::string dir)
+uintmax_t LLDiskCache::dirFileSize(const std::string& dir)
 {
     uintmax_t total_file_size = 0;
 
diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h
index 62c19361fb..f07b26c2d7 100644
--- a/indra/llfilesystem/lldiskcache.h
+++ b/indra/llfilesystem/lldiskcache.h
@@ -81,7 +81,7 @@ class LLDiskCache :
                      * a child of the main Viewer cache directory. Defined
                      * by the setting at 'DiskCacheDirName'
                      */
-                    const std::string cache_dir,
+                    const std::string& cache_dir,
                     /**
                      * The maximum size of the cache in bytes - Based on the
                      * setting at 'CacheSize' and 'DiskCachePercentOfTotal'
@@ -104,9 +104,7 @@ class LLDiskCache :
          * so many things had to be pushed back there to accomodate it, that I
          * decided to move it here.  Still not sure that's completely right.
          */
-        static const std::string metaDataToFilepath(const std::string& id,
-                                             LLAssetType::EType at);
-
+        static const std::string metaDataToFilepath(const LLUUID& id, LLAssetType::EType at);
 
         /**
          * Purge the oldest items in the cache so that the combined size of all files
@@ -141,13 +139,7 @@ class LLDiskCache :
          * directory. Primarily used here to determine the directory size
          * before and after the cache purge
          */
-        uintmax_t dirFileSize(const std::string dir);
-
-        /**
-         * Utility function to convert an LLAssetType enum into a
-         * string that we use as part of the cache file filename
-         */
-        const std::string assetTypeToString(LLAssetType::EType at);
+        uintmax_t dirFileSize(const std::string& dir);
 
     private:
         /**
diff --git a/indra/llfilesystem/llfilesystem.cpp b/indra/llfilesystem/llfilesystem.cpp
index b206aab7cf..c8ce9531c2 100644
--- a/indra/llfilesystem/llfilesystem.cpp
+++ b/indra/llfilesystem/llfilesystem.cpp
@@ -36,10 +36,10 @@
 
 #include "boost/filesystem.hpp"
 
-const S32 LLFileSystem::READ        = 0x00000001;
-const S32 LLFileSystem::WRITE       = 0x00000002;
-const S32 LLFileSystem::READ_WRITE  = 0x00000003;  // LLFileSystem::READ & LLFileSystem::WRITE
-const S32 LLFileSystem::APPEND      = 0x00000006;  // 0x00000004 & LLFileSystem::WRITE
+constexpr S32 LLFileSystem::READ        = 0x00000001;
+constexpr S32 LLFileSystem::WRITE       = 0x00000002;
+constexpr S32 LLFileSystem::READ_WRITE  = 0x00000003;  // LLFileSystem::READ & LLFileSystem::WRITE
+constexpr S32 LLFileSystem::APPEND      = 0x00000006;  // 0x00000004 & LLFileSystem::WRITE
 
 static LLTrace::BlockTimerStatHandle FTM_VFILE_WAIT("VFile Wait");
 
@@ -57,9 +57,7 @@ LLFileSystem::LLFileSystem(const LLUUID& file_id, const LLAssetType::EType file_
     if (mode == LLFileSystem::READ)
     {
         // build the filename (TODO: we do this in a few places - perhaps we should factor into a single function)
-        std::string id;
-        mFileID.asString();
-        const std::string filename = LLDiskCache::metaDataToFilepath(id, mFileType);
+        const std::string filename = LLDiskCache::metaDataToFilepath(mFileID, mFileType);
 
         // update the last access time for the file if it exists - this is required
         // even though we are reading and not writing because this is the
@@ -73,17 +71,11 @@ LLFileSystem::LLFileSystem(const LLUUID& file_id, const LLAssetType::EType file_
     }
 }
 
-LLFileSystem::~LLFileSystem()
-{
-}
-
 // static
 bool LLFileSystem::getExists(const LLUUID& file_id, const LLAssetType::EType file_type)
 {
     LL_PROFILE_ZONE_SCOPED;
-    std::string id_str;
-    file_id.toString(id_str);
-    const std::string filename = LLDiskCache::metaDataToFilepath(id_str, file_type);
+    const std::string filename = LLDiskCache::metaDataToFilepath(file_id, file_type);
 
     llifstream file(filename, std::ios::binary);
     if (file.is_open())
@@ -97,9 +89,7 @@ bool LLFileSystem::getExists(const LLUUID& file_id, const LLAssetType::EType fil
 // static
 bool LLFileSystem::removeFile(const LLUUID& file_id, const LLAssetType::EType file_type, int suppress_error /*= 0*/)
 {
-    std::string id_str;
-    file_id.toString(id_str);
-    const std::string filename =  LLDiskCache::metaDataToFilepath(id_str, file_type);
+    const std::string filename = LLDiskCache::metaDataToFilepath(file_id, file_type);
 
     LLFile::remove(filename.c_str(), suppress_error);
 
@@ -110,13 +100,8 @@ bool LLFileSystem::removeFile(const LLUUID& file_id, const LLAssetType::EType fi
 bool LLFileSystem::renameFile(const LLUUID& old_file_id, const LLAssetType::EType old_file_type,
                               const LLUUID& new_file_id, const LLAssetType::EType new_file_type)
 {
-    std::string old_id_str;
-    old_file_id.toString(old_id_str);
-    const std::string old_filename =  LLDiskCache::metaDataToFilepath(old_id_str, old_file_type);
-
-    std::string new_id_str;
-    new_file_id.toString(new_id_str);
-    const std::string new_filename =  LLDiskCache::metaDataToFilepath(new_id_str, new_file_type);
+    const std::string old_filename = LLDiskCache::metaDataToFilepath(old_file_id, old_file_type);
+    const std::string new_filename = LLDiskCache::metaDataToFilepath(new_file_id, new_file_type);
 
     // Rename needs the new file to not exist.
     LLFileSystem::removeFile(new_file_id, new_file_type, ENOENT);
@@ -127,7 +112,7 @@ bool LLFileSystem::renameFile(const LLUUID& old_file_id, const LLAssetType::ETyp
         // failed but the original code does not and doing so seems to
         // break a lot of things so we go with the flow...
         //return false;
-        LL_WARNS() << "Failed to rename " << old_file_id << " to " << new_id_str << " reason: "  << strerror(errno) << LL_ENDL;
+        LL_WARNS() << "Failed to rename " << old_file_id << " to " << new_file_id << " reason: " << strerror(errno) << LL_ENDL;
     }
 
     return true;
@@ -136,9 +121,7 @@ bool LLFileSystem::renameFile(const LLUUID& old_file_id, const LLAssetType::ETyp
 // static
 S32 LLFileSystem::getFileSize(const LLUUID& file_id, const LLAssetType::EType file_type)
 {
-    std::string id_str;
-    file_id.toString(id_str);
-    const std::string filename =  LLDiskCache::metaDataToFilepath(id_str, file_type);
+    const std::string filename = LLDiskCache::metaDataToFilepath(file_id, file_type);
 
     S32 file_size = 0;
     llifstream file(filename, std::ios::binary);
@@ -155,9 +138,7 @@ bool LLFileSystem::read(U8* buffer, S32 bytes)
 {
     bool success = false;
 
-    std::string id;
-    mFileID.toString(id);
-    const std::string filename =  LLDiskCache::metaDataToFilepath(id, mFileType);
+    const std::string filename = LLDiskCache::metaDataToFilepath(mFileID, mFileType);
 
     llifstream file(filename, std::ios::binary);
     if (file.is_open())
@@ -187,21 +168,19 @@ bool LLFileSystem::read(U8* buffer, S32 bytes)
     return success;
 }
 
-S32 LLFileSystem::getLastBytesRead()
+S32 LLFileSystem::getLastBytesRead() const
 {
     return mBytesRead;
 }
 
-bool LLFileSystem::eof()
+bool LLFileSystem::eof() const
 {
     return mPosition >= getSize();
 }
 
 bool LLFileSystem::write(const U8* buffer, S32 bytes)
 {
-    std::string id_str;
-    mFileID.toString(id_str);
-    const std::string filename =  LLDiskCache::metaDataToFilepath(id_str, mFileType);
+    const std::string filename = LLDiskCache::metaDataToFilepath(mFileID, mFileType);
 
     bool success = false;
 
@@ -212,12 +191,11 @@ bool LLFileSystem::write(const U8* buffer, S32 bytes)
         {
             ofs.write((const char*)buffer, bytes);
 
-            mPosition = (S32)ofs.tellp(); // <FS:Ansariel> Fix asset caching
+            mPosition = (S32)ofs.tellp();
 
             success = true;
         }
     }
-    // <FS:Ansariel> Fix asset caching
     else if (mMode == READ_WRITE)
     {
         // Don't truncate if file already exists
@@ -241,7 +219,6 @@ bool LLFileSystem::write(const U8* buffer, S32 bytes)
             }
         }
     }
-    // </FS:Ansariel>
     else
     {
         llofstream ofs(filename, std::ios::binary);
@@ -293,12 +270,12 @@ S32 LLFileSystem::tell() const
     return mPosition;
 }
 
-S32 LLFileSystem::getSize()
+S32 LLFileSystem::getSize() const
 {
     return LLFileSystem::getFileSize(mFileID, mFileType);
 }
 
-S32 LLFileSystem::getMaxSize()
+S32 LLFileSystem::getMaxSize() const
 {
     // offer up a huge size since we don't care what the max is
     return INT_MAX;
@@ -314,10 +291,9 @@ bool LLFileSystem::rename(const LLUUID& new_id, const LLAssetType::EType new_typ
     return true;
 }
 
-bool LLFileSystem::remove()
+bool LLFileSystem::remove() const
 {
     LLFileSystem::removeFile(mFileID, mFileType);
-
     return true;
 }
 
@@ -332,7 +308,7 @@ void LLFileSystem::updateFileAccessTime(const std::string& file_path)
      *
      * Let's start with 1 hour in time_t units and see how that unfolds
      */
-    const std::time_t time_threshold = 1 * 60 * 60;
+    constexpr std::time_t time_threshold = 1 * 60 * 60;
 
     // current time
     const std::time_t cur_time = std::time(nullptr);
diff --git a/indra/llfilesystem/llfilesystem.h b/indra/llfilesystem/llfilesystem.h
index 983e452981..10649b6920 100644
--- a/indra/llfilesystem/llfilesystem.h
+++ b/indra/llfilesystem/llfilesystem.h
@@ -38,20 +38,20 @@ class LLFileSystem
 {
     public:
         LLFileSystem(const LLUUID& file_id, const LLAssetType::EType file_type, S32 mode = LLFileSystem::READ);
-        ~LLFileSystem();
+        ~LLFileSystem() = default;
 
         bool read(U8* buffer, S32 bytes);
-        S32  getLastBytesRead();
-        bool eof();
+        S32  getLastBytesRead() const;
+        bool eof() const;
 
         bool write(const U8* buffer, S32 bytes);
         bool seek(S32 offset, S32 origin = -1);
         S32  tell() const;
 
-        S32 getSize();
-        S32 getMaxSize();
+        S32 getSize() const;
+        S32 getMaxSize() const;
         bool rename(const LLUUID& new_id, const LLAssetType::EType new_type);
-        bool remove();
+        bool remove() const;
 
         /**
          * Update the "last write time" of a file to "now". This must be called whenever a
@@ -78,8 +78,6 @@ class LLFileSystem
         S32     mPosition;
         S32     mMode;
         S32     mBytesRead;
-//private:
-//    static const std::string idToFilepath(const std::string id, LLAssetType::EType at);
 };
 
 #endif  // LL_FILESYSTEM_H
diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp
index 349df11f35..26e2d8f319 100644
--- a/indra/newview/llmeshrepository.cpp
+++ b/indra/newview/llmeshrepository.cpp
@@ -3292,8 +3292,6 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b
             // only allocate as much space in the cache as is needed for the local cache
             data_size = llmin(data_size, bytes);
 
-            // <FS:Ansariel> Fix asset caching
-            //LLFileSystem file(mesh_id, LLAssetType::AT_MESH, LLFileSystem::WRITE);
             LLFileSystem file(mesh_id, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE);
             if (file.getMaxSize() >= bytes)
             {
@@ -3302,7 +3300,6 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b
 
                 file.write(data, data_size);
 
-                // <FS:Ansariel> Fix asset caching
                 S32 remaining = bytes - file.tell();
                 if (remaining > 0)
                 {
@@ -3314,7 +3311,6 @@ void LLMeshHeaderHandler::processData(LLCore::BufferArray * /* body */, S32 /* b
                         delete[] block;
                     }
                 }
-                // </FS:Ansariel>
             }
         }
         else
@@ -3367,8 +3363,6 @@ void LLMeshLODHandler::processData(LLCore::BufferArray * /* body */, S32 /* body
         if (result == MESH_OK)
         {
             // good fetch from sim, write to cache
-            // <FS:Ansariel> Fix asset caching
-            //LLFileSystem file(mMeshParams.getSculptID(), LLAssetType::AT_MESH, LLFileSystem::WRITE);
             LLFileSystem file(mMeshParams.getSculptID(), LLAssetType::AT_MESH, LLFileSystem::READ_WRITE);
 
             S32 offset = mOffset;
@@ -3432,8 +3426,6 @@ void LLMeshSkinInfoHandler::processData(LLCore::BufferArray * /* body */, S32 /*
         && gMeshRepo.mThread->skinInfoReceived(mMeshID, data, data_size))
     {
         // good fetch from sim, write to cache
-        // <FS:Ansariel> Fix asset caching
-        //LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE);
         LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE);
 
         S32 offset = mOffset;
@@ -3483,8 +3475,6 @@ void LLMeshDecompositionHandler::processData(LLCore::BufferArray * /* body */, S
         && gMeshRepo.mThread->decompositionReceived(mMeshID, data, data_size))
     {
         // good fetch from sim, write to cache
-        // <FS:Ansariel> Fix asset caching
-        //LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE);
         LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE);
 
         S32 offset = mOffset;
@@ -3532,8 +3522,6 @@ void LLMeshPhysicsShapeHandler::processData(LLCore::BufferArray * /* body */, S3
         && gMeshRepo.mThread->physicsShapeReceived(mMeshID, data, data_size) == MESH_OK)
     {
         // good fetch from sim, write to cache for caching
-        // <FS:Ansariel> Fix asset caching
-        //LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::WRITE);
         LLFileSystem file(mMeshID, LLAssetType::AT_MESH, LLFileSystem::READ_WRITE);
 
         S32 offset = mOffset;