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.

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

Introduce abstract native SocketChannel implementation #20

Closed
wants to merge 1 commit into from

Conversation

marcuslinke
Copy link
Contributor

@headius This is the first draft. Please review. Thanks!

@headius
Copy link
Member

headius commented Oct 5, 2016

This seems fine for a start. I'm trying to decide how much should be in the abstract classes and how much should be in the child classes. Having the abstract handle the basic interaction with the fd seems appropriate. With this change, NativeSocketChannel would have almost no code in it, yes?

@marcuslinke
Copy link
Contributor Author

@headius Maybe I should move this abstract class into jnr-unixsocket for the first? I think its better reviewable in conjunction with unix domain socket stuff jnr/jnr-unixsocket#33

@felfert
Copy link

felfert commented Oct 23, 2016

👍
Seee also my comment on jnr/jnr-unixsocket#26

@@ -0,0 +1,126 @@
/*
* Copyright (C) 2008 Wayne Meissner
Copy link

Choose a reason for hiding this comment

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

@marcuslinke You probably want your name and current year here

private int fd = -1;

public AbstractNativeSocketChannel(int fd) {
this(NativeSelectorProvider.getInstance(), fd);
Copy link

@felfert felfert Oct 23, 2016

Choose a reason for hiding this comment

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

Hmm, I believe this can not work without implementing NativeSelectorProvider#openSocketChannel() which currently just throws UnsupportedOperationException. At least the open() method of SocketChannel will not work, because it uses openSocketChannel() of the supplied SelectorProvider

@marcuslinke
Copy link
Contributor Author

marcuslinke commented Oct 24, 2016

Work continues here jnr/jnr-unixsocket#33

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.

None yet

3 participants