Hi Alan, Thank you for the comments, here's the updated webrev, http://cr.openjdk.java.net/~luchsh/JDK-8043495-2/ On Mon, May 26, 2014 at 6:09 PM, Alan Bateman <Alan.Bateman@oracle.com>wrote:

On 26/05/2014 10:04, Jonathan Lu wrote:

...

Hello,

May I have following patch reviewed ?

http://cr.openjdk.java.net/~luchsh/JDK-8043495/ < http://cr.openjdk.java.net/%7Eluchsh/JDK-8043495/>

The patch will add native FileChannelImpl.transferTo0() implementation for AIX by using the 'send_file' API, http://www-01.ibm.com/support/knowledgecenter/ssw_aix_71/ com.ibm.aix.commtrf2/send_file.htm?lang=en

Is the getsockopt needed to test the destination? I don't have access to a system with AIX and the man page you cite seems to detect this and give you the ENOTSOCK.

Agree, the updated patch has removed getsockopt and will check ENOTSOCK if send_file() reports error (-1).

Otherwise I don't see any issues with this, a minor consistent issue at L245 where it could be "< 0".

Fixed in the updated patch.

Just looking at the OSX implementation just before this in the function and there is redundant ifdef __APPLE__. We could fix it with this patch or use another bug, I don't of course want to expand the scope of your change.

I'm OK with this minor removal and have done that in the second patch, please review.

-Alan

Thanks Jonathan

On Tue, May 27, 2014 at 8:49 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:

On 27/05/2014 07:31, Jonathan Lu wrote:

...

Hi Alan,

Thank you for the comments, here's the updated webrev, http://cr.openjdk.java.net/~luchsh/JDK-8043495-2/ <http://cr.openjdk.java.net/%7Eluchsh/JDK-8043495-2/>

Thanks for the updates and for including the the ifdef __APPLE__ change. With the getsockopt removed then maybe socket.h is not needed now? Otherwise looks good to me.

'send_file()' is declared in 'sys/socket.h' on AIX, so we need it. The change looks good to me as well. Thanks for contributing it! Is there any test which exercises this code? Regards, Volker

-Alan.

Thanks, Alan and Volker! I will push this change. Regards, Jonathan On Tue, May 27, 2014 at 5:00 PM, Alan Bateman <Alan.Bateman@oracle.com>wrote:

On 27/05/2014 09:49, Volker Simonis wrote:

...

: 'send_file()' is declared in 'sys/socket.h' on AIX, so we need it.

Thanks, just checking.

:

...

Is there any test which exercises this code?

The tests in java/nio/channels/FileChannel are the tests to run, they

will give the transfer methods a good work out.

-Alan.