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

Kim Barrett kim.barrett at oracle.com
Thu Apr 26 22:55:52 UTC 2018


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 readdirr 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 oslinux.inline.hpp looks fine. I was going to suggest that rather than changing perfMemorylinux.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 readdirr, 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



More information about the build-dev mailing list