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.bate...@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