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.

Sign up for GitHub

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

Jump to bottom

Adding XSetWMProtocols method to X11 #665

Merged
merged 13 commits into from
Jun 13, 2016
Merged

Adding XSetWMProtocols method to X11 #665

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
12922a4
Adding XSetWMProtocols method
zainab-ali May 30, 2016
a2c5ca6
Adding test for XSetWMProtocols
zainab-ali May 30, 2016
e1fcf5a
Adding test for XSetWMProtocols
zainab-ali May 30, 2016
25ec0b1
Adding test for XSetWMProtocols
zainab-ali May 30, 2016
391a0d2
Adding XGetWMProtocols to X11
zainab-ali Jun 3, 2016
4c85e1d
Added XSetWMProtocols and XGetWMProtocols to X11
zainab-ali Jun 3, 2016
9a963c4
Amending XGetWMProtocols for X11
zainab-ali Jun 7, 2016
f26ac18
Amending XGetWMProtocols for X11
zainab-ali Jun 7, 2016
717f7f6
Amending XGetWMProtocols for X11
zainab-ali Jun 7, 2016
f1a27b0
Merge branch 'master' of https://github.com/zainab-ali/jna
zainab-ali Jun 7, 2016
cb1c52b
Testing multiple atoms in XSetWmProtocols for X11
zainab-ali Jun 10, 2016
7bdb4fc
Merge branch 'x11'
zainab-ali Jun 10, 2016
634033c
Modifying CHANGES.md
zainab-ali Jun 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions contrib/platform/src/com/sun/jna/platform/unix/X11.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,8 @@ void XSetWMProperties(Display display, Window window, String window_name,
Pointer class_hints);

int XSetWMProtocols(Display display, Window window, Atom[] atom, int count);
Copy link
Member

@matthiasblaesing matthiasblaesing May 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This is a nice to have: Should the XGetWMProtocols function also directly implemented (looks a bit more symmetrical :-))?

int XGetWMProtocols(Display display, Window w, Atom[] protocols_return, IntByReference count_return);
Copy link
Member

Choose a reason for hiding this comment

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

This won't work - the function is declared as:

Status XGetWMProtocols(Display *display, Window w, Atom
    **protocols_return, int *count_return);

protocols_return is a pointer to an array of atoms (following the documentation). The array of atoms is allocated on the callee side and not on the java side. As an Atom is basicly a primitive long, I only see the option to bind this as a PointerByReference. From that you have to extract the data itself and in the end free the list:

        X11.Atom[] atomList = new X11.Atom[count.getValue()];
        for(int i = count.getValue() - 1; i >= 0; i--) {
            atomList[i] = new X11.Atom(protocols.getValue().getLong(X11.Atom.SIZE * i));
        }
        X11.INSTANCE.XFree(protocols.getValue());

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 - I didn't realise the array was allocated on the callee side. If I want to add a helper method, would X.java be a good place to put it?

Copy link
Member

Choose a reason for hiding this comment

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

You can see a pattern in the com.sun.platform.win32 package. In general there is the library binding (Kernel32.java) and there is the abstract helper class (Kernel32Util.java) that has "Util" as suffix and contains helpers as static methods.


int XFree(Pointer data);
Window XCreateSimpleWindow(Display display, Window parent, int x, int y,
int width, int height, int border_width,
Expand Down
15 changes: 14 additions & 1 deletion contrib/platform/test/com/sun/jna/platform/unix/X11Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.awt.GraphicsEnvironment;

import com.sun.jna.ptr.PointerByReference;
import com.sun.jna.ptr.IntByReference;

import junit.framework.TestCase;

Expand Down Expand Up @@ -76,12 +77,24 @@ public void testXFetchName() {
}
}

public void testSetWMProtocols() {
public void testXSetWMProtocols() {
X11.Atom[] protocols = new X11.Atom[]{ X11.INSTANCE.XInternAtom(display, "WM_DELETE_WINDOW", false) };
int status = X11.INSTANCE.XSetWMProtocols(display, root, protocols, 1);
Copy link
Member

Choose a reason for hiding this comment

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

Please replace the hardcoded count value of "1" with protocols.length, this enforces correct values, even if the input is changed.

assertEquals("Bad status for XSetWMProtocols", 1, status);
Copy link
Member

Choose a reason for hiding this comment

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

I read http://linux.die.net/man/3/xsetwmprotocols:

If it cannot intern the WM_PROTOCOLS atom, XSetWMProtocols returns a zero status. Otherwise, it returns a nonzero status.

So - would it be better to check for a non-zero status?

}

public void testXGetWMProtocols() {
X11.Atom[] setProtocols = new X11.Atom[]{ X11.INSTANCE.XInternAtom(display, "WM_DELETE_WINDOW", false) };
X11.INSTANCE.XSetWMProtocols(display, root, setProtocols, 1);
Copy link
Member

Choose a reason for hiding this comment

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

With regard to the hard coded count value please see comment for line 82, I'd like to see a test with multiple values.


X11.Atom[] protocols = new X11.Atom[1];
IntByReference count = new IntByReference();
int status = X11.INSTANCE.XGetWMProtocols(display, root, protocols, count);
assertEquals("Bad status for XGetWMProtocols", 1, status);
assertEquals("Wrong number of protocols returned for XGetWMProtocols", count.getValue(), 1);
assertNotNull("No protocols returned for XGetWMProtocols", protocols[0]);
}

public static void main(java.lang.String[] argList) {
junit.textui.TestRunner.run(X11Test.class);
}
Expand Down