Skip to content

Adding XSetWMProtocols method to X11#665

Merged
matthiasblaesing merged 13 commits intojava-native-access:masterfrom
zainab-ali:master
Jun 13, 2016
Merged

Adding XSetWMProtocols method to X11#665
matthiasblaesing merged 13 commits intojava-native-access:masterfrom
zainab-ali:master

Conversation

@zainab-ali
Copy link
Copy Markdown
Contributor

Adding XSetWMProtocols method to com.sun.jna.platform.unix.X11
Adding a single test to com.sun.jna.platform.unix.X11Tests to check that the command returns the correct status code.

public void testSetWMProtocols() {
X11.Atom[] protocols = new X11.Atom[]{ X11.INSTANCE.XInternAtom(display, "WM_DELETE_WINDOW", false) };
int status = X11.INSTANCE.XSetWMProtocols(display, root, protocols, 1);
assertEquals("Bad status for XSetWMProtocols", 1, status);
Copy link
Copy Markdown
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?

@matthiasblaesing
Copy link
Copy Markdown
Member

Please make sure you also add an entry into the CHANGES.md about the new function(s).

When you work on this please merge the changesets into logical segments - I'm not a fan of squashing commits just because it can be done, but it helps readability if logical blocks are commited together. In this case I think one changeset can represent the whole change without negative impact on readability.

Pointer class_hints);

int XSetWMProtocols(Display display, Window window, Atom[] atom, int count);
int XGetWMProtocols(Display display, Window w, Atom[] protocols_return, IntByReference count_return);
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@matthiasblaesing matthiasblaesing merged commit 3a803b7 into java-native-access:master Jun 13, 2016
@matthiasblaesing
Copy link
Copy Markdown
Member

@zainab-ali thank you for your contribution! I merged your changes into master. It is appreciated!

mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024
Motivation:

Due how our code was structured it was possible that we not always
handle the lifetime of the underlying quiche connection correctly.
Beside this it was also possible to convert a CLOSED channel state to an
ACTIVE again.

Modifications:

- Rewrite the state machine in QuicheQuicChannel to correctly handle the
different states and also make it impossible to change a final state
like CLOSED
- Correctly handle removal of the QuicheQuicChannel from the
datastructures in QuicheQuicChannel

Result:

Correct handling of lifetime and state.
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