Repository navigation
Add libc direct tests for unlink and symlink - #5584
Conversation
| fs::remove_file(&file_path).unwrap(); | ||
| } | ||
|
|
||
| fn test_unlink() { |
There was a problem hiding this comment.
Please also test unlinking a directory.
There was a problem hiding this comment.
I added a test for unlinking a directory.
| // Creating a symlink should succeed and point to `target`. | ||
| errno_check(unsafe { libc::symlink(c_target.as_ptr(), c_link.as_ptr()) }); | ||
| assert_eq!(fs::read_link(&link).unwrap(), target); |
There was a problem hiding this comment.
This tests creating a symlink with an absolute path, pointing to a file. Please also test
- pointing to a directory
- a relative path
There was a problem hiding this comment.
I combined the two cases into a single block because the symlink Windows branch appends the relative target to the link's directory and then checks whether the result is a directory to decide between symlink_dir and symlink_file. This logic can only be meaningfully tested using a combination of a relative path and a directory. With an absolute path, the join does nothing; with a relative path combined with a file, even an incorrect join might coincidentally yield the correct result.
There was a problem hiding this comment.
I cannot make sense of your answer. In principle there are 4 cases to consider here: relative/absolute, dir/file. Is that what you are trying to say?
There was a problem hiding this comment.
Yes, there are 4 cases. I misunderstood your comment, so I only added "relative + dir". I'll add "absolute + dir" and "relative + file".
There was a problem hiding this comment.
I have added all 4 cases.
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
|
||
| let dir_path = utils::prepare("miri_test_libc_unlink"); | ||
| fs::create_dir(&dir_path).unwrap(); | ||
| let dir_name = utils::into_c_string(&dir_path); |
There was a problem hiding this comment.
Please call this c_dir_path or so. dir_name vs dir_path to indicate the C vs Rust sring distinction is not obvious at all.
There was a problem hiding this comment.
I changed dir_name to c_dir_path.
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
rustbot
added
S-waiting-on-author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
b29e8d4 to
0eed2c8
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Thanks :) |
Part of #3179
Windows doesn't allow regular users to create symlinks by default, so I added
have_symlink_permission()to avoid test failures when the host is Windows.