Small cleanups for copy_file sendfile fix #544
Conversation
| @@ -2898,6 +2898,8 @@ namespace vcpkg | |||
| return true; | |||
| #else // ^^^ (defined(__APPLE__) // !defined(__APPLE__) vvv | |||
|
|
|||
| destination_fd.fchmod(source_stat.st_mode, ec); | |||
There was a problem hiding this comment.
This is now generally before writing the output and for some paths before reading ec, instead of immediately before returning. IMO this is a behavioural change, not deduplication.
There was a problem hiding this comment.
Perhaps, but is there a need to delay the fchmod until the end, requiring the duplication?
There was a problem hiding this comment.
Maybe it is really an improvement ;-)
There was a problem hiding this comment.
I think this change LGTM, but it needs to add a check for ec and an early failure if the fchmod failed. Failure to change permissions but success in copying the data doesn't sound like a successful copy to me :)
There was a problem hiding this comment.
Thanks that's a good point, I pushed a change.
| @@ -2898,6 +2898,8 @@ namespace vcpkg | |||
| return true; | |||
| #else // ^^^ (defined(__APPLE__) // !defined(__APPLE__) vvv | |||
|
|
|||
| destination_fd.fchmod(source_stat.st_mode, ec); | |||
There was a problem hiding this comment.
I think this change LGTM, but it needs to add a check for ec and an early failure if the fchmod failed. Failure to change permissions but success in copying the data doesn't sound like a successful copy to me :)
Cleanups for #543