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 structures and methods to enumerate sessions #1198

Merged
merged 2 commits into from
May 21, 2020

Conversation

dbwiddis
Copy link
Contributor

No description provided.

Copy link
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

I left some comments inline.

}

public SESSION_INFO_10(Pointer p) {
super(p, Structure.ALIGN_DEFAULT, W32APITypeMapper.DEFAULT);
Copy link
Member

Choose a reason for hiding this comment

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

The DEFAULT Type mapper correct here? From the header file:

//
// Only the UNICODE version of the LM APIs are available on NT.
// Non-UNICODE version on other platforms
//
#if defined( _WIN32_WINNT ) || defined( WINNT ) || defined( __midl )
|| defined( FORCE_UNICODE )
#define LMSTR LPWSTR
#define LMCSTR LPCWSTR
#else
#define LMSTR LPSTR
#define LMCSTR LPCSTR
#endif

I risk a guess, on recent Windows Versions (i.e. everything post W2K) - is WIN32_WINNT or WINNT defined? If so, this would need the UNICODE mapper, if not some more logic is necessary.

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. Those strings are always Unicode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So if they are mapped directly to WString, we don't need any type mapper.

* function fails, the return value is an error code.
*/
int NetSessionEnum(String servername, String UncClientName, String username, int level, PointerByReference bufptr,
int prefmaxlen, IntByReference entriesread, IntByReference totalentries, IntByReference resume_handle);
Copy link
Member

Choose a reason for hiding this comment

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

The string mapping might prove to be problematic, the headers are not consistent. See the differerence:

NetGetJoinInformation(
__in_opt IN LPCWSTR lpServer OPTIONAL,
__out_opt OUT LPWSTR *lpNameBuffer,
OUT PNETSETUP_JOIN_STATUS BufferType
);

In this case the function call always expects a unicode string. The same is not true for:

NetSessionEnum (
__in_opt IN LMSTR servername OPTIONAL,
__in_opt IN LMSTR UncClientName OPTIONAL,
__in_opt IN LMSTR username OPTIONAL,
IN DWORD level,
__out OUT LPBYTE *bufptr,
IN DWORD prefmaxlen,
__out OUT LPDWORD entriesread,
__out_opt OUT LPDWORD totalentries,
__inout_opt IN OUT LPDWORD resume_handle OPTIONAL
);

With the definition of LMSTR see comment in SESSION_INFO_10 for the definition. And it depends on the interpretation there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per above, LMSTR is always Unicode. So these should be WString.

public int OutgoingCompressedBytes;
public byte[] WinStationName = new byte[WINSTATIONNAME_LENGTH * CHAR_WIDTH];
public byte[] Domain = new byte[DOMAIN_LENGTH * CHAR_WIDTH];
public byte[] UserName = new byte[(USERNAME_LENGTH + 1) * CHAR_WIDTH];
Copy link
Member

Choose a reason for hiding this comment

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

I would add a javadoc comment to the three parameters, that they exist only to enable structure size calculation. I would also make the three fields final (see https://java-native-access.github.io/jna/5.5.0/javadoc/overview-summary.html -> "read-only" fields).

Reading from native memory and will still work. Even a setter should work that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding final that requires the fields be initialized in the constructor. When constructing from a Pointer this works. Should I then remove the no-arg constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually... as arrays, final doesn't really change writing to the contents. So this doesn't prevent changing the contents but does fix the length, and thus the structure offsets. Not really "read-only" but still a good idea, probably JNA-wide for arrays in structures...

public void testWTSEnumerateSessions() {
int WTSClientAddress = 14;
int WTSSessionInfo = 24;
int WTSClientProtocolType = 16;
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to move this to Wtsapi32.java as

public interface _WTS_INFO_CLASS {
    int WTSClientAddress = 14;
    int WTSClientProtocolType = 16;
    int WTSSessionInfo = 24;
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure... although I'd probably map the entire enum there. I was being a bit lazy...

@dbwiddis dbwiddis force-pushed the wts branch 2 times, most recently from 177d835 to cb2a1ba Compare May 21, 2020 06:12
@matthiasblaesing
Copy link
Member

Looks good to me. Thank you.

@matthiasblaesing matthiasblaesing merged commit 1586e6f into java-native-access:master May 21, 2020
@dbwiddis dbwiddis deleted the wts branch January 11, 2022 22:13
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