RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated

[Kim Barrett]

> On Apr 25, 2018, at 10:51 AM, Andrew Hughes <gnu.andrew at redhat.com> wrote:
> 
> On 24 April 2018 at 20:17, Kim Barrett <kim.barrett at oracle.com> wrote:
>>> On Apr 23, 2018, at 3:51 AM, Michal Vala <mvala at redhat.com> wrote:
>>> 
>>> Hi,
>>> 
>>> following discussion "RFR: build pragma error with gcc 4.4.7"[1], I'm posting updated patch replacing deprecated readdir_r with readdir. Bug ID is JDK-8179887 [2].
>>> 
>>> webrev: http://cr.openjdk.java.net/~andrew/8179887/webrev/
>>> 
>>> I'm asking for the review and eventually sponsorship.
>> 
> 
> The patch looks good to me.
> 
>> The change to os::readdir in os_linux.inline.hpp looks fine.
>> 
>> I was going to suggest that rather than changing perfMemory_linux.cpp to use os::readdir in an
>> unusual and platform-specific way, it should instead just call ::readdir directly.  However, neither
>> of those is right, and that part of the change should not be made; see
>> https://bugs.openjdk.java.net/browse/JDK-8134540
>> Much nearly duplicated code for PerfMemory support
>> 
> 
> I think, in the circumstances, Michal's solution is the best option at
> this point.
> 8134540 looks like a more long-term change currently scheduled for 12 (is
> anyone currently working on it?), whereas this fix could go into 11 and remove
> a couple of unneeded memory allocations.
> 
>> Looking a bit deeper, there might be some additional follow-on that could be done, eliminating
>> the second argument to os::readdir.
>> windows: unused
>> bsd: freebsd also deprecated readdir_r, I think for similar reasons.
>> solaris: doc is clear about thread safety issue being about sharing the DIR*
>> aix: I haven’t looked at it, but would guess it’s similar.
>> 
>> In other words, don’t operate on the DIR* from multiple threads simultaneously, and just use
>> ::readdir on non-Windows.  That would all be for another RFE though.
>> 
>> 
> 
> This also seems like a more in-depth separate change and not one that
> can be performed
> by any of us alone, as we don't test all these platforms. As it
> stands, Michal's change affects
> Linux and Linux alone, and the addition of the use of NULL will make
> it clearer that moving
> to an os:readdir is feasible on that platform.

I disagree, and still think the perfMemory_linux.cpp change should be
removed.

(1) The change to perfMemory_linux.cpp is entirely unnecessary to
address the problem this bug is about.

(2) It violates the (implied) protocol for os::readdir.  If
Linux-specific code wants to invoke Linux-specific behavior, it should
do so by calling a Linux-specific API, not abuse an intended to be
portable API.

(3) It minorly interferes with some desirable future work.  If there
were some significant benefit to doing so, I wouldn't give this much
weight, but I don't see a significant benefit.

(4) The only benefit is saving some rare short-term memory
allocations.  I don't think that's worth the above costs.

Note that the Windows version of os::readdir also ignores the second
argument, but all callers provide it anyway.

I've opened a new CR for general os::readdir cleanup.
https://bugs.openjdk.java.net/browse/JDK-8202353