griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix OSX implementation of BLI_current_working_dir and improve tests.

Browse Source 
While some implementations of `getcwd` may return an allocated string
instead of the given char buffer in some cases, this is not the expected
behavior of the BLI wrapper. Not to mention the danger of returning a
pointer to a static char buffer...

Improve `ChangeWorkingDirectoryTest` to be more 'full check' regarding
behavior of both `BLI_current_working_dir` and `BLI_change_working_dir`.
Also move call to `BLI_threadapi_init` into proper `SetUp` method (to
have correct symmetry with the call to `BLI_threadapi_exit` in the
`TearDown` one).

Based on investigation by Charles Wardlaw (@CharlesWardlaw).

Pull Request #105220
This commit is contained in:
Bastien Montagne
2023-02-26 14:42:54 +01:00
parent c4791ee009
commit a1c3061812
2 changed files with 32 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
source/blender/blenlib/intern/storage_apple.mm
View File

@@ -190,13 +190,13 @@ const char *BLI_expand_tilde(const char *path_with_tilde)
char *BLI_current_working_dir(char *dir, const size_t maxncpy) {
/* Can't just copy to the *dir pointer, as [path getCString gets grumpy.*/
static char path_expanded[PATH_MAX];
char path_expanded[PATH_MAX];
@autoreleasepool {
NSString *path = [[NSFileManager defaultManager] currentDirectoryPath];
const size_t length = maxncpy > PATH_MAX ? PATH_MAX : maxncpy;
[path getCString:path_expanded maxLength:length encoding:NSUTF8StringEncoding];
BLI_strncpy(dir, path_expanded, maxncpy);
return path_expanded;
return dir;
}
}

42
source/blender/blenlib/tests/BLI_fileops_test.cc
View File

@@ -12,6 +12,12 @@ class ChangeWorkingDirectoryTest : public testing::Test {
public:
std::string test_temp_dir;
void SetUp() override
{
/* Must use because BLI_change_working_dir() checks that we are on the main thread. */
BLI_threadapi_init();
}
void TearDown() override
{
if (!test_temp_dir.empty()) {
@@ -55,13 +61,15 @@ TEST(fileops, fstream_open_charptr_filename)
TEST_F(ChangeWorkingDirectoryTest, change_working_directory)
{
/* Must use because BLI_change_working_dir() checks that we are on the main thread. */
BLI_threadapi_init();
char original_cwd_buff[FILE_MAX];
char *original_cwd = BLI_current_working_dir(original_cwd_buff, sizeof(original_cwd_buff));
char original_wd[FILE_MAX];
if (!BLI_current_working_dir(original_wd, FILE_MAX)) {
FAIL() << "unable to get the current working directory";
}
ASSERT_FALSE(original_cwd == nullptr) << "Unable to get the current working directory.";
/* While some implementation of `getcwd` (or similar) may return allocated memory in some cases,
* in the context of `BLI_current_working_dir` usages, this is not expected and should not
* happen. */
ASSERT_TRUE(original_cwd == original_cwd_buff)
<< "Returned CWD path unexpectedly different than given char buffer.";
std::string temp_file_name(std::tmpnam(nullptr));
test_temp_dir = temp_file_name + "_новый";
@@ -79,10 +87,11 @@ TEST_F(ChangeWorkingDirectoryTest, change_working_directory)
ASSERT_TRUE(BLI_change_working_dir(test_temp_dir.c_str()))
<< "temporary directory should succeed changing directory.";
char cwd[FILE_MAX];
if (!BLI_current_working_dir(cwd, FILE_MAX)) {
FAIL() << "unable to get the current working directory";
}
char new_cwd_buff[FILE_MAX];
char *new_cwd = BLI_current_working_dir(new_cwd_buff, sizeof(new_cwd_buff));
ASSERT_FALSE(new_cwd == nullptr) << "Unable to get the current working directory.";
ASSERT_TRUE(new_cwd == new_cwd_buff)
<< "Returned CWD path unexpectedly different than given char buffer.";
#ifdef __APPLE__
/* The name returned by std::tmpnam is fine but the Apple OS method
@@ -91,12 +100,21 @@ TEST_F(ChangeWorkingDirectoryTest, change_working_directory)
test_temp_dir = "/private" + test_temp_dir;
#endif // #ifdef __APPLE__
ASSERT_EQ(BLI_path_cmp_normalized(cwd, test_temp_dir.c_str()), 0)
ASSERT_EQ(BLI_path_cmp_normalized(new_cwd, test_temp_dir.c_str()), 0)
<< "the path of the current working directory should equal the path of the temporary "
"directory that was created.";
ASSERT_TRUE(BLI_change_working_dir(original_wd))
ASSERT_TRUE(BLI_change_working_dir(original_cwd))
<< "changing directory back to the original working directory should succeed.";
char final_cwd_buff[FILE_MAX];
char *final_cwd = BLI_current_working_dir(final_cwd_buff, sizeof(final_cwd_buff));
ASSERT_FALSE(final_cwd == nullptr) << "Unable to get the current working directory.";
ASSERT_TRUE(final_cwd == final_cwd_buff)
<< "Returned CWD path unexpectedly different than given char buffer.";
ASSERT_EQ(BLI_path_cmp_normalized(final_cwd, original_cwd), 0)
<< "The final CWD path should be the same as the original CWD path.";
}
} // namespace blender::tests