RFR(XS): 8231885: Fix/remove malformed assert in os_windows.cpp

[Thomas Stüfe]

Hi Christoph,

Fix is fine. I agree semantically the assert in this place is not a good
choice.

assert(x) (with one parm) binds to the C standard assert introduced via
<assert.h> or <cassert.h>. My guess would be we pull cassert.h in jdk/jdk
and don't in 11u.

assert(x) is active unless NDEBUG is defined. As a precaution, we should
make sure we define NDEBUG in release builds, do deactivate any C-Runtime
asserts which may slip in because someone forgets the second assert
argument (eg. when copy-pasting code from jdk to hotspot). Just my 5 cents
:)

Cheers, Thomas

On Fri, Oct 4, 2019 at 3:28 PM Langer, Christoph <christoph.langer at sap.com>
wrote:

> Hi,
>
> please review a one(two)-line fix for a recent change in os_windows.cpp.
>
> The fix introduced a malformed assertion which doesn't provide an
> assertion message. Testing in jdk/jdk wouldn't bring up the issue but I
> just discovered it when attempting to backport it to jdk11. Probably due to
> the older compiler or some other thing that is different there the
> fastdebug build failed.
>
> One could add a message to the assertion. But I think handing a malformed
> path that starts with 3 or more '\' to wide_abs_unc_path will either cause
> an error code and the caller should be able to cope with that or windows is
> tolerant enough to accept it as a UNC path still. We can leave that to the
> operating system. So I decided to simply remove the assertion.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8231885
> Problematic statement:
> http://hg.openjdk.java.net/jdk/jdk/file/13f29c43b6c7/src/hotspot/os/windows/os_windows.cpp#l4192
>
> Fix:
>
> diff -r e25b317d0350 -r 3f89a4aa0c2d src/hotspot/os/windows/os_windows.cpp
> --- a/src/hotspot/os/windows/os_windows.cpp     Thu Oct 03 18:59:56 2019
> +0100
> +++ b/src/hotspot/os/windows/os_windows.cpp     Fri Oct 04 14:11:47 2019
> +0100
> @@ -4189,8 +4189,6 @@
>      if (::isalpha(buf[0]) && !::IsDBCSLeadByte(buf[0]) && buf[1] == ':'
> && buf[2] == '\\') {
>        prefix = L"\\\\?\\";
>      } else if (buf[0] == '\\' && buf[1] == '\\') {
> -      assert(buf[2] != '\\');
> -
>        if (buf[2] == '?' && buf[3] == '\\') {
>          prefix = L"";
>          needs_fullpath = false;
>
>
> Thanks
> Christoph
>
>