Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asdk Contrib - Issue #2111 Abs Paths through proxy #2112

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cozdas
Copy link
Collaborator

@cozdas cozdas commented Jan 17, 2025

Fixing issue #2111

  • Added fallback mechanism for files with absolute paths so that if the proxy doesn't provide hash or the content, the file system will be used. We still expect the proxy to provide files with relative paths.

  • Removed closeLutStream() function as both the ifstream and the stringstream objects that getLutData may return use RAII idiom. Since we already use unique pointers to hold the objects, we don't need to worry about closing the file or freeing memory.

  • Added OCIOZ unit tests for existing and non-existing files with absolute paths.

cozdas added 4 commits January 9, 2025 10:12
- Do not use config proxy for absolute paths while computing file hash or loading LUT data.
- Added the unit test provided in the ticket.

Signed-off-by: cuneyt.ozdas <[email protected]>
…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]>
…hy windows build is configured to use c++14+ while other platforms use C++11. Replacing make_unique with the new syntax to make the other platforms happy too.

Signed-off-by: cuneyt.ozdas <[email protected]>
- Added a test for absolute path to inexistent file.

Signed-off-by: cuneyt.ozdas <[email protected]>

if (!filestream.good())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not good if pStream becomes null somehow. Sticking with the pointer and adding a null check instead of using a reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant