Looks good!
On Thu, Dec 17, 2015 at 12:14 AM, vyom <vyom.tew...@oracle.com> wrote: > Hi Martin, > > thanks for review, i undated the > webrev(http://cr.openjdk.java.net/~vtewari/4823133/webrev0.2/ > <http://cr.openjdk.java.net/%7Evtewari/4823133/webrev0.2/>) as per your > comment after confirming that the corresponding "fd" if opened with > "open64". > > Thanks, > Vyom > > > > On Thursday 17 December 2015 12:38 AM, Martin Buchholz wrote: >> >> Calling naked fstat without _FILE_OFFSET_BITS=64 is a bug. Don't you >> need to call fstat64 if it's available? >> >> +jlong >> +handleGetLength(FD fd) >> +{ >> + struct stat sb; >> + if (fstat(fd, &sb) == 0) { >> + return sb.st_size; >> + } else { >> + return -1; >> + } >> +} >> >> On Wed, Dec 16, 2015 at 5:02 AM, vyom <vyom.tew...@oracle.com> wrote: >>> >>> Hi, >>> >>> Updated the webrev(http://cr.openjdk.java.net/~vtewari/4823133/webrev0.1/ >>> <http://cr.openjdk.java.net/%7Evtewari/4823133/webrev0.1/>) as per Martin >>> review comment removed the _FILE_OFFSET_BITS from source. >>> >>> Thanks, >>> Vyom >>> >>> >>> >>> On Tuesday 15 December 2015 10:55 PM, Martin Buchholz wrote: >>>> >>>> _FILE_OFFSET_BITS is generally an all-or-nothing thing, because it >>>> affects interoperability between translation units. It would be good >>>> to convert all of the JDK build to use -D_FILE_OFFSET_BITS=64, but >>>> that would be a big job. So traditionally the JDK has instead used >>>> the functions made available via _LARGEFILE64_SOURCE. But that is >>>> also a JDK-wide job now, because every call to plain stat in the >>>> source code is broken on 32-bit systems with 64-bit inodes, which are >>>> becoming more common. >>>> >>>> I recommend the _FILE_OFFSET_BITS=64 strategy, but it's probably a job >>>> for build-dev, not core-libs-dev. >>>> >>>> >>>> >>>> >>>> On Tue, Dec 15, 2015 at 8:31 AM, Roger Riggs <roger.ri...@oracle.com> >>>> wrote: >>>>> >>>>> Hi Yvom, >>>>> >>>>> Minor comments: >>>>> >>>>> src/java.base/share/native/libjava/RandomAccessFile.c: >>>>> - "length fail" might be clearer as "GetLength failed" >>>>> >>>>> src/java.base/unix/native/libjava/io_util_md.c: >>>>> >>>>> - Please add a comment before the define of FILE_OFFSET_BITS to >>>>> indicate >>>>> where it is used and why it is there. >>>>> - BTW, are there any unintended side effects? >>>>> Perhaps a different issue but perhaps 64 bit offsets should be >>>>> used >>>>> everywhere >>>>> >>>>> src/java.base/windows/native/libjava/io_util_md.c >>>>> - Line 592: Using INVALID_HANDLE_VALUE is better than -1 and is used >>>>> elsewhere in the file >>>>> BTW, Testing for invalid handle might be unnecessary since the >>>>> call >>>>> to >>>>> GetFileSizeEx will fail >>>>> if it is invalid, yielding the same result. >>>>> >>>>> Roger >>>>> >>>>> >>>>> On 12/10/2015 5:52 AM, vyom wrote: >>>>>> >>>>>> Hi All, >>>>>> >>>>>> Please review my changes for below bug. >>>>>> >>>>>> Bug: JDK-4823133 : RandomAccessFile.length() is not thread-safe >>>>>> >>>>>> Webrev:http://cr.openjdk.java.net/~vtewari/4823133/webrev0.0/ >>>>>> <http://cr.openjdk.java.net/%7Evtewari/4823133/webrev0.0/> >>>>>> >>>>>> This change ensure that length() does not temporarily changes the >>>>>> file >>>>>> pointer and it will make sure that there is no race >>>>>> condition in case of multi thread uses. >>>>>> >>>>>> Thanks, >>>>>> Vyom >>>>>> >>>>>> >>>>>> >>>>>> >
