-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 missing PRINTER_INFO_X structs #1429
base: master
Are you sure you want to change the base?
Conversation
…ll 1-9 structs identically.
*/ | ||
@FieldOrder({"pSecurityDescriptor"}) | ||
public static class PRINTER_INFO_3 extends Structure { | ||
public WinNT.SECURITY_DESCRIPTOR_RELATIVE pSecurityDescriptor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although WinNT
provides a struct named SECURITY_DESCRIPTOR
, the struct is malformed for use with PRINTER_INFO_3
despite having an identical name. Using WinNT.SECURITY_DESCRIPTOR
will throw a runtime error java.lang.IllegalStateException: Array fields must be initialized
.
Fortunately SECURITY_DESCRIPTOR_RELATIVE
matches the struct design exactly that's returned by PRINTER_INFO_3
, fixing this runtime exception. This is just an FYI and can be acknowledged by a project maintainer if there are no objections. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pointer to SECURITY_DESCRIPTOR
and I believe the _RELATIVE
version is the correct "by reference" mapping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reopening... so going back on the DWORD
vs int
conversation, I'm looking for examples and found that PRINTER_INFO_2
uses public INT_PTR pSecurityDescriptor;
however this PR uses the WinNT.SECURITY_DESCRIPTOR_RELATIVE
. Should I switch PRINTER_INFO_3
to match? It seems less useful, but I'd like to be consistent. If so, how would I go about getting the data, construct a Pointer
from the int and manually cast it? If that's the case, is this better than just using the correct struct to begin with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue occurs for PRINTER_INFO_2
, which returns a INT_PTR
in place of the pDevMode
, whereas the newly written PRINTER_INFO_8
, PRINTER_INFO_9
both use this PR's struct. pDevMode
appears again in L_PRINTER_DEFAULTS
, but this time as a Pointer
object.
I can update PRINTER_INFO_2
, but those using the API in its current form will break.
Alternately I can switch PRINTER_INFO_9
and PRINTER_INFO_8
to use Pointers and then allow DEVMODE
to be constructed manually, but I think this will be less intuitive.
I tried to convert the INT_PTR
to a Pointer and then to a struct, but I got an error java.lang.UnsupportedOperationException: This pointer is opaque: const@0x1a31bba904c
.
// Doesn't work :(
public DEVMODE(Pointer p) {
super(p);
ensureAllocated();
read();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reopening... so going back on the
DWORD
vsint
conversation, I'm looking for examples and found thatPRINTER_INFO_2
usespublic INT_PTR pSecurityDescriptor;
however this PR uses theWinNT.SECURITY_DESCRIPTOR_RELATIVE
. Should I switchPRINTER_INFO_3
to match?
My apologies, I answered the first one thinking about a completely different recent change. Ultimately you need to map a pointer of some sort, and then use that pointer to map to the structure it points to. Using the ByReference
tag on the structure accomplishes this transparently and should probably be your first choice. So I'd suggest WinNT.SECURITY_DESCRIPTOR_RELATIVE.ByReference
. Check the commit dates on the other structure, it's entirely possible it was committed first.
Don't change old ones to break compatibility. If you want consistency, feel free to match them, but INT_PTR
is a pointer-sized integer so you'd need to use toPointer()
to convert it and then use that pointer to pass to a constructor.
It seems less useful, but I'd like to be consistent. If so, how would I go about getting the data, construct a
Pointer
from the int and manually cast it? If that's the case, is this better than just using the correct struct to begin with?
I think the structure (by reference) is the easiest. So you get INT_PTR ip
, you do Pointer p = ip.toPointer()
and then pass p
to the pointer constructor of the structure, essentially this (FooStructure extends Structure
):
FooStructure(Pointer p) {
super(p);
// anything else you need to do
}
"dmPelsHeight", "dummyunionname2", "dmDisplayFrequency", "dmICMMethod", "dmICMIntent", "dmMediaType", "dmDitherType", | ||
"dmReserved1", "dmReserved2", "dmPanningWidth", "dmPanningHeight" }) | ||
|
||
public static class DEVMODE extends Structure { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reopening this comment, since the previous one at this code marking ended up talking about fixing the pointer and then reading a wide string properly.
Quoting @dbwiddis:
Unions are fine, but generally you should try to add code in the constructor that detects which union member needs to be read, and then reads it. That looks a little bit complex here... do you read fields and then based on which bits are set choose which union member applies?
@dbwiddis Is the current structure OK? My structure is a copy from a project called "damage" and it seems to mirror the Microsoft definition exactly, take note of the words DUMMY...
in the definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the current structure OK?
I'd quibble over style; I'd prefer to see all the structure fields together at the top, and the nested class definitions below them. Right now it's hard to see which elements are in the union.
Yes, it's fine to match the Windows API names.
There is one missing piece, though, and I'm not clear how the structure works. In the native, all fields of the union are essentially "active" at any given time. In JNA, we only "read" the member of the union which is declared to be the correct type. For most unions this information is given in one of the other fields.
In this structure it looks like one of the fields is a bitmask indicating which of several union fields may be active. So you might have to do some bit manipulation there, or perhaps just read all three possibilities, doing something like this in the DEVMODE
structure:
@Override
public void read() {
super.read();
dmUnion1.setType(DUMMYSTRUCTNAME.class);
dmUnion1.read();
dmUnion1.setType(POINT.class);
dmUnion1.read();
dmUnion1.setType(DUMMYSTRUCTNAME2.class);
dmUnion1.read();
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbwiddis thanks. I've taken a deep dive into DEVMODE
and I'm starting to understand its data. I'm not entirely sure what happens when it's of unexpected length and that's something I need to do more research on.
For starters, I've placed JavaDoc comments into the Structure explaining exactly when a field is used, so this PR has a lot more DEVMODE information in it now.
I've also moved the fields around, I hope they're to your liking. :)
-
dmDriverExtra
: For example, the private driver datadmDriverExtra
is meaningless to JNA, it can only ever be used by the driver manufacturer, since this format is intentionally proprietary. I'm sure someone will eventually write a function to read this, but it may be easier to just extend theDEVMODE
struct when that happens. I'm fine leaving this alone, we just won't read the private data, as we have no place to put it. :) -
dmSize
: I think this one is going to be our friend, but I haven't wrapped my head around how to use it. What I've researched so far...DUMMYUNIONNAME.DUMMYSTRUCTNAME
is NEVER used for Displays. For this reason, I would like to rename it, but I don't know the ramifications of this.DUMMYUNIONNAME.DUMMYSTRUCTNAME2
equally is NEVER used for Printers. For this reason, I would like to rename it as well, if that's a sane thing to do.
*Correction, this is a shared struct, used for both printers and displays.- I have a suspicion that
dmSize
will give us some insight into whichDUMMYSTRUCT
will be provided, but I would love some help in this area if possible. I can't find a bitwise flags (yet) that would just expose to use if this DEVMODE is of type PRINTER or DISPLAY, but I think (I hope) that this answers the original concerns. - One final option is I could make a skeleton DEVMODE interface and extend/implement it for DEVMODE_PRINTER, DEVMODE_DISPLAY, however I think this is a bit of overkill and would be less intuitive to the end-user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First ... not sure what you're doing with the XP_OR_HIGHER
boolean. If you do need it, use the Windows API VersionHelpers.IsWindowsXPOrGreater()
.
To me it looks like dmSize
would always be the same value which would match the JNA-calculated size()
of this structure. I don't think you can use it for choosing the union -- by definition the largest potential member is always used. It appears dmDriverExtra
can give a clue about a larger allocation you could use, but you have to first read the structure once to get that value, allocate something bigger, and then read it again to be able to use it. Not sure that's a use case you need to handle, though.
It looks to me like you could use the dmFields
value. The example in the API mentions the bit dmOrientation
which is in the DUMMYSTRUCTNAME
struct. So if that bit is present, you'd have to read that member of the union. Same for dmPaperSize
, dmPaperLength
, etc. So you'd modify my code above to something like:
@Override
public void read() {
super.read();
if (dmFields & (DM_ORIENTATION | DM_PAPERSIZE | DM_PAPERLENGTH
| DM_PAPERWIDTH | DM_SCALE | DM_COPIES | DM_DEFAULTSOURC
| DM_PRINTQUALITY) > 0) {
dmUnion1.setType(DUMMYSTRUCTNAME.class);
dmUnion1.read();
}
if (dmFields & (DM_POSITION) > 0) {
dmUnion1.setType(POINT.class);
dmUnion1.read();
}
if (dmFields & (DM_POSITION | DM_DISPLAYORIENTATION | DM_DISPLAYFIXEDOUTPUT) > 0) {
dmUnion1.setType(DUMMYSTRUCTNAME2.class);
dmUnion1.read();
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading more it does look like there are different versions of the structure, thus the different version numbers and size fields available. I'm guessing the newer version(s) added/would add fields at the end. So if the size is smaller than size()
you would only partially read the structure.
So this falls into a typical Windows API situation of "call once to get the size and then call again with the properly sized buffer" pattern.
Hey @tresf, are you going to continue working on this PR? I think you've done a lot of good work here and just need a few tweaks to get it merged. |
@dbwiddis yes thanks for asking and for all of your help so far. I will be resuming work on this in a few weeks. I have a feature that requires some of the new enums, so I would like to see this feature added in a future release. |
Does it make sense to keep this open? It is now dormant for more than a year. |
Due to resource reasons, I haven't been able to work on this, but I will eventually finish it. It's fine if you prefer to clean up the tracker. |
@tresf no problem, was just checking |
Devmodes seem a bit strange when it comes to their size. In this example https://learn.microsoft.com/en-us/troubleshoot/windows/win32/modify-printer-settings-documentproperties (scroll to bottom) we see they are allocating a section of memory, then casting it to a devmode. /*
* Step 1:
* Allocate a buffer of the correct size.
*/
dwNeeded = DocumentProperties(hWnd,
hPrinter, /* Handle to our printer. */
pDevice, /* Name of the printer. */
NULL, /* Asking for size, so */
NULL, /* these are not used. */
0); /* Zero returns buffer size. */
pDevMode = (LPDEVMODE)malloc(dwNeeded); When I ran the example, I was seeing mallocs of 1689, while the devmodeA was only 156 bytes long. I assume the end result of this is a devmode with a lot of extra data written to the right of it, then to access that data, you would need to cast it to a larger struct or maybe do some pointer manipulation. How would we approach this over-allocation of a structure with jna? Add a Byte[] field to the end? |
Yep, docs for DocumentProperties() say
Nope, keep the DEVMODE structure as-is, but follow the usual JNA pattern of "query once to get size, allocate, and query again". Per that same link:
So call DocumentProperties to get the size, create a |
Interesting, this was @Vzor- 's first attempt. It doesn't work, the Memory object is almost always too small. I assumed this was caused by stack alignment. Any ideas? |
This is a JNA-originated error associated with trying to assign something in your structure out of bounds. I just looked at your mappings again and it seems your
When you say "seems to complete" does that mean the call had a successful return value? In that case the error from GetLastError may be leftover from the initial call to find the size. |
Unfortunately this isn't the case (GetLastError is being called after each call). Yes, a successful call still shows this, but what's more concerning is the Memory allocation. As @Vzor- has explained, he has to over-allocate the Memory object to avoid the bounds exceeded. This is without the DEVMODE in the codebase at all. I'll ask him to share the code, it's a pretty simple snippet. |
"Bounds exceeds available space" is a JNA-produced message when you are trying to read something from, or write to, a jna/src/com/sun/jna/Memory.java Line 222 in da87d77
Well, something is being read from or written to the That error message would not occur just calling |
Hmm... thanks for the sanity check. Although we can confirm this is true with malloc in C, in Java, it's not enough. @Vzor- has the small small snippet, but if he uses the return value from the first call to set the size on Memory object for the second, it's never big enough. It always requires a size that's 1-3 more. The additional size varies based on the printer. The same does not occur using the struct, but the struct needs to be fixed first (it's causing intermittent crashing, likely due to previous discussions) |
Hmm.. testing locally and this problem doesn't occur. We'll keep digging. There may be something up with @Vzor-'s machine. Here's the gist I'm working from: https://gist.github.com/tresf/f5fc8cc19037fece6230461bda7c13ac. With exception of the final call to GetLastError, everything (including Memory allocation) is just fine. I'm not sure what this suggests about @Vzor-'s machine though. Note, I'm testing on Windows 11 ARM64 edition, he's on Intel. |
Check your mapping for the |
The docs say:
|
Yeah, I just edited that.... try
do longer printer names result in longer bytes required? |
Sure, where? The existing function definitions (e.g. When @Vzor- and I crawled the web for this, most people just omitted the printer name to In regards to the predictable |
OK, had to dig into this a bit, and you're probably fine with the String. Both
Yeah I'd already seen that and summarized it with my statement "the error from GetLastError may be leftover from the initial call to find the size." If the second call returns successfully, where is the 122 error coming from? I still feel like I'm trying to debug with incomplete information here. The "Bounds exceeds available space" error message should have come with a stack trace. That only happens when trying to read or write to a Nothing in the gist that I see would have caused either of the errors, so there's not much help we can provide you debugging. |
Right, but as indicated, we're checking both times.
I'm not aware of what this is. Do you mind offering a hint?
Besides the 122, which is most likely benign, the errors don't occur on my machine. (Sorry if I did not make this clear or obvious). I had reproduced the code from memory awaiting feedback from @Vzor-, quoting:
|
From |
You should not check GetLastError after a success unless the API specifically says you should. Usually it only says to call it after failure for more information. It's not reset after the previous time and depending on the API, it may just have a leftover value from a previous call.
See the comment in the example in the FAQ: public interface MyUser32 extends User32 {
// DEFAULT_OPTIONS is critical for W32 API functions to simplify ASCII/UNICODE details
MyUser32 INSTANCE = (MyUser32)Native.load("user32", W32APIOptions.DEFAULT_OPTIONS);
void ThatFunctionYouReallyNeed();
} Did you include the |
This is essentially what the comments in the S.O. article state. Thanks for the confirmation.
No, I've been using JNA for quite a while and never used this. Thanks! |
Slight correction, when I cloned |
I discovered the issue causing the out of bounds exception. I was dumping the memory as chars and my for loop wasn't checking that there were 2 bytes (since the chars are wide) or more remaining. This means it would crash when reading odd length Memory objects. |
The documentation does not state whether the strings in DEVMODE are \0 terminated so assume they might be not
I'm late to this discussion, so this might already be to late, but I noticed some things and I pushed a test branch here: https://github.com/matthiasblaesing/jna/tree/devmode The relevant changes:
With these changes the manual test gives me sane values. |
I am making the assumption that the two name fields are null terminated. I have seen data overwritten in the fields, and the old name was not completely removed, just written over with a null terminated string. Windows also limits printer name to 31 chars, for a 32 char long buffer. public String getDmDeviceName() {
long offset = fieldOffset("dmDeviceName");
if(CHAR_WIDTH == 1) {
//todo: this can overrun if there is no null, perhaps we should add overrun protection to getString
return this.getPointer().getString(offset);
} else {
return this.getPointer().getWideString(offset);
}
} It would be nice if there were an overload for Pointer.getString that included some kind of overrun protection. Something like |
This should be an safe alternative: public String getDmDeviceName() {
long offset = fieldOffset("dmDeviceName");
if(CHAR_WIDTH == 1) {
//todo: this can overrun if there is no null, perhaps we should add overrun protection to getString
return Native.toString(this.getPointer().getByteArray(offset, dmDeviceName.length));
} else {
return Native.toString(this.getPointer().getCharArray(offset, dmDeviceName.length / CHAR_WIDTH));
}
} |
Adds to Winspool.java:
PRINTER_INFO_3
,PRINTER_INFO_5
,PRINTER_INFO_7
,PRINTER_INFO_8
,PRINTER_INFO_9
Adds to WinGDI.java:
CCHFORMNAME=32
DEVMODE
(see TODO)Please see in-line comments below. 🍻
TODO:
byte[] dmDeviceName
🗨️ Discussion herePRINTER_INFO_
Javadocs in similar style to existing classDEVMODE
versusDEVMODEA
🗨️ Discussion hereDWORD
) or Java types (e.g. 'int`) 🗨️ Discussion hereDWORD
toint
, etc. 🗨️ Discussion hereDUMMYSTRUCTNAME
🗨️ Discussion here🗨️ Discussion here