Hi Chris!

A couple of minor comments.

1)
if (s != -1 || (s == -1 && errno != EINVAL)) {

This can be simplified to

if (s != -1 || errno != EINVAL) {

2)
What about sockets created with accept(): Shouldn't NET_Accept be modified to set O_CLOEXEC as well?
On Linux accept4() can be used to pass SOCK_CLOEXEC flag.

With kind regards,
Ivan

On 7/25/18 5:49 AM, Chris Hegarty wrote:
Alan,

On 25 Jul 2018, at 08:24, Alan Bateman <alan.bate...@oracle.com> wrote:

...

As I said previously, the patch isn't complete so native code calling fork/exec 
may still have to deal with other file descriptors that are inherited into the 
child. I don't object to doing this in phases of course but somehow we have 
managed to get by for 20 years without this being an issue.
I added the following to the JIRA description to make this clear:

"This JIRA issue, by itself, does not completely resolve the problem
of native code calling fork/exec, it may still have to deal with other
file descriptors that are inherited by the child. Instead this JIRA
issue is targeted at the socket and channels areas only, other areas
should be tackled on a phased approach though separate JIRA
issues."

The updates to the various site to use the NET_* functions are fine. However, I 
think the new functions in net_util_md.c could be cleaner. I think it would be 
better to fallback to socket/socketpair + fcntl when the initial call fails 
with EINVAL.
Agreed. How about this ( trying to reduce the ifdef blocks, and
keep them relatively clean ) :

---
JNIEXPORT int JNICALL
NET_Socket(int domain, int type, int protocol) {
     int s;
#if defined(SOCK_CLOEXEC)
     s = socket(domain, type | SOCK_CLOEXEC, protocol);
     if (s != -1 || (s == -1 && errno != EINVAL)) {
         return s;
     }
#endif
     s = socket(domain, type, protocol);
     if (s != -1) {
         // Best effort - return value is intentionally ignored since the socket
         // was successfully created.
         fcntl(s, F_SETFD, FD_CLOEXEC);
     }
     return s;
}
---

Updated webrev:
   http://cr.openjdk.java.net/~chegar/8207335/webrev.01/

-Chris.

--
With kind regards,
Ivan Gerasimov

Reply via email to