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 :) > -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Web Site: http://fuseyism.com Twitter: https://twitter.com/gnu_andrew_java PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
