Implement file cloning on Windows #4344
Conversation
|
|
||
| if this.ptr_is_null(target_handle_ptr)? { | ||
| throw_unsup_format!( | ||
| "`DuplicateHandle` `lpTargetHandle` parameter is null, which is unsupported" |
There was a problem hiding this comment.
The official docs say
If lpTargetHandle is NULL, the function duplicates the handle, but does not return the duplicate handle value to the caller. This behavior exists only for backward compatibility with previous versions of this function. You should not use this feature, as you will lose system resources until the target process terminates.
So unsupported is correct, but that doesn't have to be in the error message. The message could just say that the pointer must not be null because the handle must be written somewhere or it would get leaked
There was a problem hiding this comment.
I wasn't sure if we wanted to consider this case 'unsupported by Miri' or 'viewed as a bug by Miri'. I'm fine with considering it to be a program bug.
There was a problem hiding this comment.
yea it's a weird balance. "unsupported" hints at "may add support", which considering the official docs I would say we won't do
| "`DuplicateHandle` called on a thread handle, which is unsupported" | ||
| ); | ||
| } | ||
| Handle::Pseudo(pseudo) => Handle::Pseudo(pseudo), |
There was a problem hiding this comment.
Do we have tests for this code path?
There was a problem hiding this comment.
No, we don't currently directly test any of the handle shims outside of ones used directly for filesystem stuff. I can add such tests when I get a chance.
There was a problem hiding this comment.
yea may be good to test these code paths, but also preexisting, so nothing urgent
|
As a follow-up, could you add a test directly calling the shim ( I'm trying to ensure as many shims as possible have at least smoke tests invoking them directly and not via std so that we are more robust against std implementation changes. :) |
Just requires expanding what we can do with handles - get the current process pseudo-handle, and the Windows version of
dup.