On 24/04/2018 1:27 PM, Andrew Hughes wrote:
On 23 April 2018 at 20:19, Kim Barrett <kim.barr...@oracle.com> wrote:
On Apr 21, 2018, at 11:18 AM, Andrew Hughes <gnu.and...@redhat.com> wrote:

On 19 March 2018 at 23:23, Kim Barrett <kim.barr...@oracle.com> wrote:
There are also problems with the patch as provided.

(1) Since PRAGMA_DIAG_PUSH/POP do nothing in the version of gcc this
change is being made in support of, the warning would be disabled for
all following code in any translation unit that includes this file.
That doesn't seem good.

No, but it's really the only solution on those compilers. We have such
usage already elsewhere e.g.

// Silence -Wformat-security warning for fatal()
PRAGMA_DIAG_PUSH
PRAGMA_FORMAT_NONLITERAL_IGNORED
  fatal(buf);
PRAGMA_DIAG_POP
  return true; // silence compiler warnings
}
in src/hotspot/os_cpu/linux_zero/os_linux_zero.cpp

If there are other warnings, then they will picked up on newer compilers,
especially when building with -Werror. I don't think it's likely people are
doing development on older compilers, but rather that we have to use
them to build for older platforms.

I would be a lot more comfortable if the possibly do-nothing push/pop and
the associated code were in a .cpp file, rather than in a .hpp file where it
could affect some open-ended and unexpected set of code.


Ah yes, sorry, I missed that this was a .hpp, while the others were .cpp.

But I think this is moot if os::readdir can be changed to call ::readdir rather
than ::readdir_r, as appears to be the case, possibly with some documentation
about not sharing the DIR* among multiple threads, at least not without locking.


I think so too for OpenJDK 11, but I'm reluctant to backport such a change
to older JDKs.
I guess if we want to continue to workaround the warning there, we'll need
to move the function into the .cpp file.

That seemed to be what Michal was planning to do, but hasn’t gotten back to
it yet.

Indeed. He has a patch that does that, that I've already reviewed. Just waiting
for him to post it publicly :)

He already has:

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

on both the mailing lists this email has gone to.

David
-----





Reply via email to