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

Add WTSEnumerateProcessesEx and WTS_PROCESS_INFO_EX to Wtsapi32 #981

Closed
wants to merge 2 commits into from

Conversation

dbwiddis
Copy link
Contributor

@dbwiddis dbwiddis commented Jul 1, 2018

No description provided.

@FieldOrder({ "SessionId", "ProcessId", "pProcessName", "pUserSid", "NumberOfThreads", "HandleCount",
"PagefileUsage", "PeakPagefileUsage", "WorkingSetSize", "PeakWorkingSetSize", "UserTime", "KernelTime" })
class WTS_PROCESS_INFO_EX extends Structure {
public DWORD SessionId;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to define this as int? Same goes for other DWORDs introduced here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment I just noted on another PR. I went back and forth a few times between int and DWORD. What sold me on the latter was reading a message thread about Windows Process IDs, and seeing a comment from a Microsoft Engineer that they actually had seen ProcessID values above Integer.MAX_VALUE. Is there a general policy we follow for JNA mappings? I understand primitives would be more efficient.

Copy link
Member

Choose a reason for hiding this comment

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

Your argument is sound, see the other PR for my comment on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm ok leaving it as is unless/until the JNA project decides to convert ALL the code. As-is, it's somewhat of a mixed bag (I've seen both styles in existing code) and users must generally be responsible. Might be a good subject to put in the Contributing document.

class WTS_PROCESS_INFO_EX extends Structure {
public DWORD SessionId;
public DWORD ProcessId;
public Pointer pProcessName; // Either LPSTR or LPWSTR
Copy link
Member

Choose a reason for hiding this comment

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

This can be mapped as String, if you specify the correct type mapper - see Sspi#SecPkgInfo for a sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny, I originally had (and still have) it as a String in my project, and it worked, but I didn't know the inner workings well enough to know if it handled the Unicode/ANSI conversions. I'll put it back!

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if you already found the "explanation". If not this is the short version: The default Windows function mapper maps to either the ASCII or the UNICODE version of a function (if they follow the windows conventions) based on the value of the w32.ascii system property. The same logic is then applied in com.sun.jna.win32.W32APITypeMapper.W32APITypeMapper(boolean). A string is on the fly converted to a WString and on the return way converted back to a string.

* the function fails, the return value is zero. To get extended
* error information, call the GetLastError function.
*/
boolean WTSFreeMemoryEx(int WTSTypeClass, Pointer pMemory, long NumberOfEntries);
Copy link
Member

Choose a reason for hiding this comment

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

This should explode! NumberOfEntries is a ULONG and according to the MSDN that is 32 bit long and not 64bit:
https://docs.microsoft.com/en-us/windows/desktop/winprog/windows-data-types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Interestingly, this was one of my first JNA mappings a few years ago, and I obviously made some errors then, most that I've fixed. But it's been in my project's code and hasn't caused a (known) problem yet. Obviously needs fixing but may be a case where byte boundaries are saving my skin.

Copy link
Member

Choose a reason for hiding this comment

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

I've been there and was burned by it - searching for something like this for a few hours sharpens sensitivity for this :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants