`Netapi32Util#getDCName(String, String)` has switched arguments for `NetGetDCName` call

[Marcono1234]

Version

02bf7cc

Description

In the class com.sun.jna.platform.win32.Netapi32Util the method getDCName(String, String) seems to have accidentally (?) switched arguments:
It calls Netapi32#NetGetDCName with (domainName, serverName, ...) but that method expects (String serverName, String domainName, ...) (which is also the signature of the native Windows function it seems).

jna/contrib/platform/src/com/sun/jna/platform/win32/Netapi32Util.java

Line 121 in 02bf7cc

int rc = Netapi32.INSTANCE.NetGetDCName(domainName, serverName, bufptr);

Maybe this hasn't been noticed yet because the method seems to only be used within JNA with (null, null) as arguments so it would not detect switched arguments. A search on GitHub doesn't show any usages of Netapi32Util#getDCName outside of JNA either.

Unfortunately I am neither familiar enough with this project nor with the native Windows function to verify this.
Hopefully this information is useful nonetheless, but feel free to close this issue if you don't think it is useful.

[matthiasblaesing]

Thank you. Yes this looks broken and is a candiate for fixing.

Looking at the function not only are the arguments switched, it is also not freeing the returned string. While the documentation is lacking this crucial information, the example is clearly using NetApiBufferFree to free the string. This should be fixed aswell, if anyone takes a look.

[Marcono1234]

it is also not freeing the returned string

I think this is done in the enclosing try-finally, which calls NetApiBufferFree. But that is a bit difficult to read or notice because the call to NetApiBufferFree is on the right side of the if condition.
Maybe similar to the other methods it should have a separate rc variable for that instead, for better readability?

int rc = Netapi32.INSTANCE.NetApiBufferFree(...);
...

Though maybe it should also check for NULL before trying to free memory? Though not sure what the correct approach here is, some of the other methods in this class check getPointer() != null and others check getValue() != Pointer.NULL.

[matthiasblaesing]

@Marcono1234 you are right. I overlooked it in the finally-clause. So that leaves the switched elements and the potential null check.