Skip to content
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

Changing Net User things to use ffi instead of win32-api #3344

Merged
merged 14 commits into from
May 15, 2015
Merged

Conversation

jaym
Copy link
Contributor

@jaym jaym commented May 7, 2015

This was a mess of memory corruption, so I've rewritten using the saner ffi approach. It now works with 64-bit rubies.

cc @chef/client-windows @adamedx @ksubrama

else
val.to_wstring
end
val = FFI::MemoryPointer.from_string(encoded)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're setting val = directly here, you don't need the val = on line 88 or the else block below.

@ksubrama
Copy link

👍 Loading a library to fetch error messages is too insane for this pull-request. Leaving that for another time. The rest of this looks awesome!

LOGON32_PROVIDER_DEFAULT = 0
LOGON32_LOGON_NETWORK = 3
LOGON32_PROVIDER_DEFAULT = Security::LOGON32_PROVIDER_DEFAULT
LOGON32_LOGON_NETWORK = Security::LOGON32_LOGON_NETWORK
#XXX for an extra painful alternative, see: http://support.microsoft.com/kb/180548
def validate_credentials(passwd)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in retrospect, this should have been something like valid_credentials? but wutevs. #breakfastwhenevs

@btm
Copy link
Contributor

btm commented May 15, 2015

👍 nice work man. it's a big change, so let's hope we don't have any regressions where we failed to have tests before.

the functional tests pass on my 8.1 workstation too 🚀.

@smurawski
Copy link
Contributor

👍 Looks good and I can make tests pass. Squash before merge?

@jaym
Copy link
Contributor Author

jaym commented May 15, 2015

I think I'm ok with the commits (except the last one)... they tell the story I want them to and each one should be working point in time

@smurawski
Copy link
Contributor

cool beans

jaym added a commit that referenced this pull request May 15, 2015
Changing Net User things to use ffi instead of win32-api
@jaym jaym merged commit eaffc0a into master May 15, 2015
@jaym jaym deleted the jdm/netuser branch May 15, 2015 16:40
jaym added a commit that referenced this pull request May 15, 2015
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants