(PUP-7627) Changes CreateSymbolicLinkW return from BOOL to BOOLEAN #5953
Conversation
I was tracking down why CreateSymbolicLinkW was returning non-zero when failing on one test VM and not a different one (puppetlabs/puppetlabs_spec_helper#192). Turns out CreateSymbolicLinkW returns a `BOOLEAN` (1 byte) rather than a `BOOL` (4 bytes), so I was getting random garbage in the upper 3 bytes and therefore a non-zero result.
rodjek
force-pushed
the
win32_bool_vs_boolean
branch
from
54a554e
to
e0bd386
Compare
|
CLA signed by all contributors. |
|
Great catch @rodjek - thanks! I'm a bit surprised we never ran into this previously. Can you retarget this at stable? This should go into the next LTS .z |
| @@ -165,6 +165,9 @@ def read_com_memory_pointer(&block) | |||
| # https://blogs.msdn.com/b/oldnewthing/archive/2011/03/28/10146459.aspx | |||
| FFI.typedef :int32, :win32_bool | |||
|
|
|||
| # The BOOLEAN data type (different to BOOL) is only 1 byte. | |||
| FFI.typedef :int8, :boolean | |||
There was a problem hiding this comment.
This is not quite right as the Win32 BYTE is unsigned and the FFI :int8 is signed per https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751%28v=vs.85%29.aspx.
A little further in the file we define :byte (and map it to the FFI :uchar) - so I'd suggest we use that same mapping... https://github.com/ffi/ffi/wiki/Types
FFI.typedef :uchar, :boolean
|
Could you also update the signature at https://github.com/puppetlabs/puppet/blob/master/lib/puppet/feature/base.rb#L83 ? While we never actually call the declared function there, it would still be good to update for consistency. Thanks again! |
| @@ -397,7 +397,7 @@ def self.resolve_symlink(handle) | |||
| begin | |||
| ffi_lib :kernel32 | |||
| attach_function_private :CreateSymbolicLinkW, | |||
| [:lpwstr, :lpwstr, :dword], :win32_bool | |||
| [:lpwstr, :lpwstr, :dword], :boolean | |||
There was a problem hiding this comment.
I have compared against the only place where we check the result of the call. The docs say returns 0 if failed, which is what we're checking (though we're using the constant WIN32_FALSE which was originally created to compare against BOOL and not BOOLEAN).
That said, I think we're fine with what we've got.
|
I fixed the minor issues and retargeted at stable in #5958, so closing this. Thanks! |
|
Thanks @Iristyle! |
I was tracking down why CreateSymbolicLinkW was returning non-zero when
failing on one test VM and not a different one
(puppetlabs/puppetlabs_spec_helper#192).
Turns out CreateSymbolicLinkW returns a
BOOLEAN(1 byte) rather than aBOOL(4 bytes), so I was getting random garbage in the upper 3 bytesand therefore a non-zero result.