RFR: 8202353: os::readdir should use readdir instead of readdir_r

[coleen.phillimore at oracle.com]

This looks very nice but one question:

http://cr.openjdk.java.net/~kbarrett/8202353/open.00/src/hotspot/os/linux/os_linux.cpp.udiff.html

+ while (result && (ptr = readdir(dir)) != NULL) {


What deallocates the dirent* memory?

thanks,
Coleen

On 7/10/18 4:18 PM, Kim Barrett wrote:
> Please review this change to the various os::readdir implementations
> for POSIX platforms to no longer use readdir_r, but instead use
> readdir.  readdir_r has been deprecated by glibc and at least some BSD
> variants, and seems to have been a mistake from the get-go.  See the
> readdir_r Portability Note in recent versions of the glibc
> documentation, e.g.
> https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html.
>
> Because we're no longer using readdir_r, the second argument for
> os::readdir is now unused by any platform's implementation.  So we've
> removed that argument, as well as the associated helper function
> os::readdir_buf_size().  All callers have been updated to no longer
> allocate and pass along the buffer for the second argument.
>
> As part of this, merged all the (now identical) non-Windows
> implementations of os::opendir, os::readdir, and os::closedir into
> os_posix.cpp.  This means the os wrappers are no longer inlined, but
> these aren't performance critical.
>
> This also eliminated some "approximations" of the proper size for the
> dirent argument that weren't correct (c.f. the deprecation rationale).
> These places should have been using readdir_buf_size(), which would
> have forced some of them to use C heap instead of stack allocation.
>
> Eliminating the 2nd argument for os::readdir required fixing some uses
> in jfr.  Some implementations of
> SystemProcessInterface::SystemProcesses::ProcessIterator assumed the
> second argument would always contain the same information as the
> result, when the result is non-NULL.  But that was not part of the
> os::readdir contract, and was never true for the Windows port (though
> the Windows port uses a different mechanism for getting process info).
> With JDK-8179887 it is no longer true for the Linux port either, which
> broke some things in jfr; see JDK-8202835.  That bug is being fixed as
> part of changing jfr for the new os::readdir signature, and the
> relevant test is being reinstated.
>
> Also updated various unqualified calls to opendir/closedir that were
> actually calling the dirent.h functions directly to instead use the os
> wrappers, for consistency.
>
> Note that this is undoing the readdir-related parts of JDK-4647546.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8202353
> https://bugs.openjdk.java.net/browse/JDK-8202835
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8202353/open.00/
>
> Testing:
> mach5 tier1-5.
> tier5 is where TestSystemProcess.java is run.
> There are a number of jfr tests in tier3-4 that use process iterators on
> relevant platforms.
> There are tests related to PerfCounters scattered through tier1-3.
>