Cleanup: Remove redundant writability check in librt/cache.c

[spargy4050v]

This PR removes the redundant cache directory writable check in src/librt/cache.c.

The cache directory writable permission were already validated in the function rt_cache_prep() before calling the cache_try_store(), making the additional check inside cache_try_store() unnecessary.

The build completed successfully, and the cache tests passed using "ctest -R cache".

Please let me know if any changes are needed.

[Copilot]

Pull request overview

This PR removes a cache directory permission check from cache_try_store() in src/librt/cache.c, claiming it is redundant with validation already performed in rt_cache_prep() via cache_init().

Key Changes:

• Removed the writable and executable permission check on cache->dir from cache_try_store()
• Removed associated error handling and resource cleanup code

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The removed code checked both writable AND executable permissions on cache->dir. However, cache_init() only checks for writable permission (line 269), not executable permission. On Unix systems, execute permission on a directory is required to traverse into it and access files within it. Without this check, the code may attempt to write to a cache directory that has write but not execute permission, which will fail. The executable permission check for cache->dir should be added to cache_init() before removing this check, or this check should not be removed.

[spargy4050v]

Agree that it will removing executable check without relocating would be incorrect. So, I've paused further changes and getting in touch with the original author on zulip, to better understand the cache design. I'll update the PR once intent is clarified.

[spargy4050v]

Hi @starseeker

Since execute permission on a directory is required to traverse into it, removing the check entirely may be incorrect unless that permission is checked before.

Going forward now, I see two possible solutions for this and I want to know which aligns better with the organization's expectations.

1. Move the executable permission to cache_init() and keep the cache_try_store() free of permission checks
2. Keep the executable permission check only inside cache_try_store(), as writable permission is already checked earlier.

Please let me know which approach you would prefer and I will update my PR accordingly.