-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
cp: fix recursive socket file copy #8463
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
Conversation
|
GNU testsuite comparison: |
|
Thanks! MacOS and FreeBSD are unhappy ,-( |
drinkcat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to press submit review...
| #[cfg(unix)] | ||
| pub fn make_socket(path: &Path) -> std::io::Result<()> { | ||
| let name = CString::new(path.to_str().unwrap()).unwrap(); | ||
| let err = unsafe { mknod(name.as_ptr(), S_IFSOCK, 0) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you still set permissions? e.g. S_IFSOCK | 0o666?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw permissions are set from the source file after file creation. So I thought I could skip them here or did I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's tricky, it depends on the exact set of flags passed to cp I believe.
| log_info("mksocket", &full_path); | ||
| unsafe { | ||
| let socket_name: CString = CString::new(full_path).expect("CString creation failed."); | ||
| libc::mknod(socket_name.as_ptr(), libc::S_IFSOCK, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question about permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
|
The issue seems to be that mknod requires super user privileges in freebsd/macos. I will open a new pull request with necessary changes. |
You can just add commits here. No need to create a new PR. Also, it would probably be acceptable to skip those tests on FreeBSD/MacOS if needed. |
|
My bad :( I already opened a pull request #8478 . Had to change the implementation. |
Fixes #8378