- Changing the logic so that for abs paths we first try the configPro…

…xy and if that fails fall back to file system. For relative paths, we don't fall back to file system though, proxy is expected to handle those. - Removed the unnecessary closeLutStream() function. We're using unique pointers, that means RAII is in place. The whole idea behind RAII is we don't need to worry about the cleanup or the type of the object wrapped by the RAII handler (unique_ptr in this case). - Cleaned up some unnecessary conversions, type shuffling and copies around the code I touched. - Cleaned up some unsafe type casts which are prone to dereferencing null pointers. Signed-off-by: cuneyt.ozdas <[email protected]>
autodesk-forks · Jan 11, 2025 · dcfdeff · dcfdeff
1 parent 7eb3d4f
commit dcfdeff
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 47 deletions.
diff --git a/src/OpenColorIO/PathUtils.cpp b/src/OpenColorIO/PathUtils.cpp
@@ -88,10 +88,16 @@ std::string GetFastFileHash(const std::string & filename, const Context & contex
             fileHashResultPtr->ready = true;
 
             std::string h = "";
-            if (!pystring::os::path::isabs(filename) && context.getConfigIOProxy())
+            if (context.getConfigIOProxy())
             {
                 // Case for when ConfigIOProxy is used (callbacks mechanism).
                 h = context.getConfigIOProxy()->getFastLutFileHash(filename.c_str());
+
+                // For absolute paths, if the proxy does not provide a hash, try the file system.
+                if (h.empty() && pystring::os::path::isabs(filename)) 
+                {
+                    h = g_hashFunction(filename);
+                }
             }
             else
             {

diff --git a/src/OpenColorIO/transforms/FileTransform.cpp b/src/OpenColorIO/transforms/FileTransform.cpp
@@ -190,37 +190,35 @@ std::unique_ptr<std::istream> getLutData(
     const std::string & filepath, 
     std::ios_base::openmode mode)
 {
-    if (!pystring::os::path::isabs(filepath) && config.getConfigIOProxy())
+    if (config.getConfigIOProxy())
     {
-        std::vector<uint8_t> buffer = config.getConfigIOProxy()->getLutData(filepath.c_str());
-        std::stringstream ss;
-        ss.write(reinterpret_cast<const char*>(buffer.data()), buffer.size());
-
-        return std::unique_ptr<std::stringstream>(
-            new std::stringstream(std::move(ss))
-        );
-    }
-
-    // Default behavior. Return file stream.
-    return std::unique_ptr<std::ifstream>(
-        new std::ifstream(Platform::filenameToUTF(filepath).c_str(), mode)
-    );
-}
+        std::vector<uint8_t> buffer;
+        // Try to open through proxy.
+        try 
+        {
+            buffer = config.getConfigIOProxy()->getLutData(filepath.c_str());
+        } 
+        catch (const std::exception&) 
+        {
+            // If the path is absolute, we'll try the file system. but otherwise
+            // nothing to do.
+            if (!pystring::os::path::isabs(filepath)) 
+              throw;
+        }
 
-// Close stream returned by getLutData
-void closeLutStream(const Config & config, const std::istream & istream)
-{
-    // No-op when it is using ConfigIOProxy since getLutData returns a vector<uint8_t>.
-    if (config.getConfigIOProxy() == nullptr)
-    {
-        // The std::istream is coming from a std::ifstream.
-        // Pointer cast to ifstream and then close it.
-        std::ifstream * pIfStream = (std::ifstream *) &istream;
-        if (pIfStream->is_open())
+        // if the buffer is empty, we'll try the file system for abs paths.
+        if (!buffer.empty() || !pystring::os::path::isabs(filepath)) 
         {
-            pIfStream->close();
+            auto pss = std::make_unique<std::stringstream>();
+            pss->write(reinterpret_cast<const char*>(buffer.data()), buffer.size());
+
+            return pss;
         }
     }
+
+    // Default behavior. Return file stream.
+    return std::make_unique<std::ifstream>(
+        Platform::filenameToUTF(filepath), mode);
 }
 
 bool CollectContextVariables(const Config &, 
@@ -635,9 +633,8 @@ void LoadFileUncached(FileFormat * & returnFormat,
                 filepath, 
                 tryFormat->isBinary() ? std::ios_base::binary : std::ios_base::in 
             );
-            auto & filestream = *pStream;
 
-            if (!filestream.good())
+            if (!pStream || !pStream->good())
             {
                 std::ostringstream os;
                 os << "The specified FileTransform srcfile, '";
@@ -647,7 +644,7 @@ void LoadFileUncached(FileFormat * & returnFormat,
                 throw Exception(os.str().c_str());
             }
 
-            CachedFileRcPtr cachedFile = tryFormat->read(filestream, filepath, interp);
+            CachedFileRcPtr cachedFile = tryFormat->read(*pStream, filepath, interp);
 
             if(IsDebugLoggingEnabled())
             {
@@ -660,17 +657,10 @@ void LoadFileUncached(FileFormat * & returnFormat,
             returnFormat = tryFormat;
             returnCachedFile = cachedFile;
 
-            closeLutStream(config, filestream);
-
             return;
         }
         catch(std::exception & e)
         {
-            if (pStream)
-            {
-                closeLutStream(config, *pStream); 
-            }
-
             primaryErrorText += "    '";
             primaryErrorText += tryFormat->getName();
             primaryErrorText += "' failed with: ";
@@ -712,9 +702,8 @@ void LoadFileUncached(FileFormat * & returnFormat,
                 filepath, 
                 altFormat->isBinary() ? std::ios_base::binary : std::ios_base::in 
             );
-            auto& filestream = *pStream;
 
-            if (!filestream.good())
+            if (!pStream || !pStream->good())
             {
                 std::ostringstream os;
                 os << "The specified FileTransform srcfile, '";
@@ -725,7 +714,7 @@ void LoadFileUncached(FileFormat * & returnFormat,
                 throw Exception(os.str().c_str());
             }
 
-            cachedFile = altFormat->read(filestream, filepath, interp);
+            cachedFile = altFormat->read(*pStream, filepath, interp);
 
             if(IsDebugLoggingEnabled())
             {
@@ -738,17 +727,10 @@ void LoadFileUncached(FileFormat * & returnFormat,
             returnFormat = altFormat;
             returnCachedFile = cachedFile;
 
-            closeLutStream(config, filestream);
-
             return;
         }
         catch(std::exception & e)
         {
-            if (pStream)
-            {
-                closeLutStream(config, *pStream); 
-            }
-
             if(IsDebugLoggingEnabled())
             {
                 std::ostringstream os;