Added WinioctlUtil, Ntifs and modified Kernel32

[amarcionek]

• Added com.sun.jna.platform.win32.WinioctlUtil for help in determining FSCTL_* codes
• Added com.sun.jna.platform.win32.Ntifs with Reparse Point structures and defines
• Added initialization of FILETIME from LARGE_INTEGER
• Added GetFileInformationByHandleEx and SetFileInformationByHandle to com.sun.jna.platform.win32.Kernel32

[matthiasblaesing]

Thank you - this looks very good. Comments:

• Place the new features at the end of the list in CHANGES.md (they are more or less in chronical order with the newest at the end)
• Adjust the license header to mention Apache License and LGPL v2 or later (see https://github.com/java-native-access/jna/blob/master/www/Contributing.md#copyright-headers-in-files)
• Privilege class: what happens if you need multiple privileges? If I read this correctly you are droping the thread token in the error case, so if one of multiple privileges fails, all are dropped? One sulution might be to allow the Privilege class to wrap multiple privileges. Or could you please describe how this should work? (I'm not that deep inside the w32 api)
• The datatypes in the new structures are the windows specifics. I'm curious if you really need these. The windows definitions are pretty mostly explicit with regard to their bit size. USHORT could be replaced with short, ULONG with int and so on. The user then has to remember, that one needs to be aware, that the value on the java side could overflow and toUnsignedInt/toUnsignedLong need to be used.
  
  This question is caused by an issue raised here, conserning excessive object creation.

[matthiasblaesing]

@amarcionek I just remembered, that you asked on the jna-user mailing list about mapping of LPTSTR. Please review the structures and function mappings you introduced. Kernel32 is mapped like this:

Kernel32 INSTANCE = Native.loadLibrary("kernel32", Kernel32.class, W32APIOptions.DEFAULT_OPTIONS);

DEFAULT_OPTIONS is set dependend on the system property w32.ascii (see W32APIOptions.java in jna-core). These options set two relevant items:

• the function mapper (this makes it possible to call FindFirstFile, even if the two functions are called FindFirstFileW (Unicode) and FindFirstFileA (ANSI))
• the type mapper, which maps String to either a char[] or a wchar[]

This needs to be addressed you mapped:

HANDLE FindFirstFile(LPWSTR /*MSDN: LPCTSTR*/ lpFileName, Pointer lpFindFileData);

So I'd map this as:

HANDLE FindFirstFile(String lpFileName, Pointer lpFindFileData);

else the moment someone sets w32.ascii to true this blows.

For the structures please also check which mapping is right a TCHAR depends on how it was created, if called from a unicode function it will be a WCHAR and from an ascii function it will be CHAR. Map the structure members to String and pass in the correct typemapper: W32APITypeMapper.DEFAULT

I had to go through a pile of wrong structure mappings here:

ab84488

If I remember correctly these were also mixed. some structures contained LPTSTR (calling dependend string), some LPWSTR (always unicode).

[amarcionek]

Hi @matthiasblaesing,

Thanks for reviewing my PR. I'm working on some of the items now. One comment on the Privilege class:

You are correct that the thread token is being reset, in both the error case in enable() AND in the disable() function. We have used this class to enable a singular privilege for a short duration for a 1 or 2 function calls as needed. The idea is to enable the privilege, do your work, and then disable it immediately after. If you need multiple Privileges, then you'd need a class instance for each one. If you need multiple for a short duration and one fails, then arguably you can't complete the work within that block. You are correct it doesn't take into account impersonating in a code block elsewhere where the thread's token is already set. It was really designed to help remove the boilerplate.

For multiples, I could see adding functionality to support more than one privilege and have them all on or off. I'd be happy to do that, or simply remove it and add that boilerplate into the test that needs it.

[matthiasblaesing]

Thanks - one datatype adjustment is needed UCHAR can't be mapped to char, but needs to be byte:

https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#UCHAR
https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#CHAR

With regard to the Privilege class: I'm not that deep inside the privilege handling of windows - is it common or uncommon to need multiple privileges? If it is common I would see multiple privileges to be dealt with atomicly as a benefit.

One other suggestion, as you pointed out, that you want to keep the privileged blocks small: Let Privilege implement Closable and map close to disable. If enable return the object itself this will become possible:

Privilege privilegeSet = new Privilege("SE_INC_BASE_PRIORITY_NAME", "SE_INCREASE_QUOTA_NAME");
try(Privilege p1 = privilegeSet.enable()) {
       // Do work with privilegeSet enabled
}
// privilegeSet disabled again

With java 9 this should work:

Privilege privilegeSet = new Privilege("SE_INC_BASE_PRIORITY_NAME", "SE_INCREASE_QUOTA_NAME");
try(privilegeSet.enable()) {
       // Do work with privilegeSet enabled
}
// privilegeSet disabled again

[amarcionek]

I believe the only UCHAR that needed to be changed from char to byte was the one in the compression structure.

For Privilege, I implemented the ability to set multiple ones via a passed in String array.
I also implemented Closeable. However, your example code for the testing threw warnings about privilegeSet not being closed because its instantiated outside the try with sources statement. So what I did was add a parameter to the constructor to enable it automatically and the whole object can be instantiated inside the try with sources statement. If programmers decide not to enable it immediately, they will be responsible for calling close().

[matthiasblaesing]

• Compilation of JNA happens in a pre JDK7 environment - please don't use try-with-resource in unittests
  Implementing closeable is possible, but not usage of it in the try-initalization block
• Make Constructor of Privilege varargs: Privilege(String... privileges) (If you want to keep the boolean argument you can move it to front). This makes the instantiation shorter and better readable
• Instead of directly enabling to make the try-with-resource call possible I suggest an alternative:
  Keep the constructor simple and don't acquire privilege, but let enable return the privilege this(!). That way this becomes possible:

public class Privilege {
  //...
  public Privilege enable() {
        // current implementation of enable
        return this;
  }

  //...
}

try(Privilege enabledPrivilege = privilege.enable()) {
     // do something 
}

[amarcionek]

I've made those changes. But I did run into one issue: the initial privilege gets a compiler warning if used in a try with resources: Resource leak: 'privilege' is never closed

Code:

Privilege privilege = new Privilege(WinNT.SE_ASSIGNPRIMARYTOKEN_NAME, WinNT.SE_BACKUP_NAME); try(Privilege privilegeEnabled = privilege.enable()) { // Will throw if it fails p.enable() fails }

Even though we know privilegeEnabled is the same one, the compiler doesn't. Now, for JDK 7 we can't use try with resources, so for the unit test I added an explicit close in the finally block.

[matthiasblaesing]

@amarcionek Thank you Adam! I just merged the changeset into master. Please check this suggested change. This turns the WinioctlUtil members into final static ints. These can be "import static`ed". WinioctlFunction looks like an interface in search of a function (:-))

diff --git a/contrib/platform/src/com/sun/jna/platform/win32/WinioctlUtil.java b/contrib/platform/src/com/sun/jna/platform/win32/WinioctlUtil.java
index 5475dde..d92d49f 100644
--- a/contrib/platform/src/com/sun/jna/platform/win32/WinioctlUtil.java
+++ b/contrib/platform/src/com/sun/jna/platform/win32/WinioctlUtil.java
@@ -43,43 +43,33 @@
 		return ((DeviceType) << 16) | ((Access) << 14) | ((Function) << 2) | (Method);
 	}
 
-	/**
-	 * Base interface for a Winiotcl function used to construct the control code
-	 */
-	public interface WinioctlFunction {
-		/**
-		 * @return the control code given the IOTCL's parameters
-		 */
-		public abstract int getControlCode();
-	}
+	public static final int FSCTL_GET_COMPRESSION = CTL_CODE(
+                Winioctl.FILE_DEVICE_FILE_SYSTEM, 
+                Winioctl.FSCTL_GET_COMPRESSION,  
+                Winioctl.METHOD_BUFFERED, 
+                Winioctl.FILE_ANY_ACCESS);
 
-	public static class FSCTL_GET_COMPRESSION implements WinioctlFunction {
-		public int getControlCode() {
-			return WinioctlUtil.CTL_CODE(Winioctl.FILE_DEVICE_FILE_SYSTEM, Winioctl.FSCTL_GET_COMPRESSION,  Winioctl.METHOD_BUFFERED, Winioctl.FILE_ANY_ACCESS);
-		}
-	}
+	public static final int FSCTL_SET_COMPRESSION = CTL_CODE(
+                Winioctl.FILE_DEVICE_FILE_SYSTEM, 
+                Winioctl.FSCTL_SET_COMPRESSION,  
+                Winioctl.METHOD_BUFFERED, 
+                WinNT.FILE_READ_DATA | WinNT.FILE_WRITE_DATA);
 
-	public static class FSCTL_SET_COMPRESSION implements WinioctlFunction {
-		public int getControlCode() {
-			return WinioctlUtil.CTL_CODE(Winioctl.FILE_DEVICE_FILE_SYSTEM, Winioctl.FSCTL_SET_COMPRESSION,  Winioctl.METHOD_BUFFERED, WinNT.FILE_READ_DATA | WinNT.FILE_WRITE_DATA);
-		}
-	}
+	public static final int FSCTL_SET_REPARSE_POINT = CTL_CODE(
+                Winioctl.FILE_DEVICE_FILE_SYSTEM, 
+                Winioctl.FSCTL_SET_REPARSE_POINT,  
+                Winioctl.METHOD_BUFFERED, 
+                Winioctl.FILE_SPECIAL_ACCESS);
 
-	public static class FSCTL_SET_REPARSE_POINT implements WinioctlFunction {
-		public int getControlCode() {
-			return WinioctlUtil.CTL_CODE(Winioctl.FILE_DEVICE_FILE_SYSTEM, Winioctl.FSCTL_SET_REPARSE_POINT,  Winioctl.METHOD_BUFFERED, Winioctl.FILE_SPECIAL_ACCESS);
-		}
-	}
+	public static final int FSCTL_GET_REPARSE_POINT = CTL_CODE(
+                Winioctl.FILE_DEVICE_FILE_SYSTEM, 
+                Winioctl.FSCTL_GET_REPARSE_POINT,  
+                Winioctl.METHOD_BUFFERED, 
+                Winioctl.FILE_ANY_ACCESS);
 
-	public static class FSCTL_GET_REPARSE_POINT implements WinioctlFunction {
-		public int getControlCode() {
-			return WinioctlUtil.CTL_CODE(Winioctl.FILE_DEVICE_FILE_SYSTEM, Winioctl.FSCTL_GET_REPARSE_POINT,  Winioctl.METHOD_BUFFERED, Winioctl.FILE_ANY_ACCESS);
-		}
-	}
-
-	public static class FSCTL_DELETE_REPARSE_POINT implements WinioctlFunction {
-		public int getControlCode() {
-			return WinioctlUtil.CTL_CODE(Winioctl.FILE_DEVICE_FILE_SYSTEM, Winioctl.FSCTL_DELETE_REPARSE_POINT,  Winioctl.METHOD_BUFFERED, Winioctl.FILE_SPECIAL_ACCESS);
-		}
-	}
+	public static final int FSCTL_DELETE_REPARSE_POINT = CTL_CODE(
+                Winioctl.FILE_DEVICE_FILE_SYSTEM, 
+                Winioctl.FSCTL_DELETE_REPARSE_POINT,  
+                Winioctl.METHOD_BUFFERED, 
+                Winioctl.FILE_SPECIAL_ACCESS);
 }
\ No newline at end of file
diff --git a/contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java b/contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java
index b0ca34c..5ae6720 100644
--- a/contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java
+++ b/contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java
@@ -62,10 +62,10 @@
 import com.sun.jna.platform.win32.WinNT.MEMORY_BASIC_INFORMATION;
 import com.sun.jna.platform.win32.WinNT.OSVERSIONINFO;
 import com.sun.jna.platform.win32.WinNT.OSVERSIONINFOEX;
-import com.sun.jna.platform.win32.WinioctlUtil.FSCTL_GET_COMPRESSION;
-import com.sun.jna.platform.win32.WinioctlUtil.FSCTL_GET_REPARSE_POINT;
-import com.sun.jna.platform.win32.WinioctlUtil.FSCTL_SET_COMPRESSION;
-import com.sun.jna.platform.win32.WinioctlUtil.FSCTL_SET_REPARSE_POINT;
+import static com.sun.jna.platform.win32.WinioctlUtil.FSCTL_GET_COMPRESSION;
+import static com.sun.jna.platform.win32.WinioctlUtil.FSCTL_GET_REPARSE_POINT;
+import static com.sun.jna.platform.win32.WinioctlUtil.FSCTL_SET_COMPRESSION;
+import static com.sun.jna.platform.win32.WinioctlUtil.FSCTL_SET_REPARSE_POINT;
 import com.sun.jna.ptr.IntByReference;
 import com.sun.jna.ptr.ShortByReference;
 
@@ -587,7 +587,7 @@
             IntByReference lpBytes = new IntByReference();
 
             if (false == Kernel32.INSTANCE.DeviceIoControl(hFile,
-                    new FSCTL_GET_COMPRESSION().getControlCode(),
+                    FSCTL_GET_COMPRESSION,
                     null,
                     0,
                     lpBuffer.getPointer(),
@@ -602,7 +602,7 @@
             lpBuffer = new ShortByReference((short)WinNT.COMPRESSION_FORMAT_LZNT1);
 
             if (false == Kernel32.INSTANCE.DeviceIoControl(hFile,
-                    new FSCTL_SET_COMPRESSION().getControlCode(),
+                    FSCTL_SET_COMPRESSION,
                     lpBuffer.getPointer(),
                     USHORT.SIZE,
                     null,
@@ -613,7 +613,7 @@
             }
 
             if (false == Kernel32.INSTANCE.DeviceIoControl(hFile,
-                    new FSCTL_GET_COMPRESSION().getControlCode(),
+                    FSCTL_GET_COMPRESSION,
                     null,
                     0,
                     lpBuffer.getPointer(),
@@ -666,7 +666,7 @@
                 REPARSE_DATA_BUFFER lpBuffer = new REPARSE_DATA_BUFFER(WinNT.IO_REPARSE_TAG_SYMLINK, (short) 0, symLinkReparseBuffer);
 
                 assertTrue(Kernel32.INSTANCE.DeviceIoControl(hFile,
-                        new FSCTL_SET_REPARSE_POINT().getControlCode(),
+                        FSCTL_SET_REPARSE_POINT,
                         lpBuffer.getPointer(),
                         lpBuffer.getSize(),
                         null,
@@ -677,7 +677,7 @@
                 Memory p = new Memory(REPARSE_DATA_BUFFER.sizeOf());
                 IntByReference lpBytes = new IntByReference();
                 assertTrue(Kernel32.INSTANCE.DeviceIoControl(hFile,
-                        new FSCTL_GET_REPARSE_POINT().getControlCode(),
+                        FSCTL_GET_REPARSE_POINT,
                         null,
                         0,
                         p,

[amarcionek]

Thanks! I will get on that now. You are absolutely correct.