RFR(S): 8188122: Path length limits on Windows leads to obscure class loading failures
Calvin Cheung
calvin.cheung at oracle.com
Tue Nov 21 16:41:47 UTC 2017
Thanks, Ioi.
Calvin
On 11/21/17, 8:36 AM, Ioi Lam wrote:
> Looks good. Thanks!
>
> - Ioi
>
>
> On 11/20/17 12:05 PM, Calvin Cheung wrote:
>> I've had some off-list discussion with Ioi resulting in another
>> update to the webrev.
>>
>> - added relative path scenario to the test. Currently, this fix
>> doesn't handle relative path on windows yet. The following RFE has
>> been filed to cover long relative paths on windows:
>> https://bugs.openjdk.java.net/browse/JDK-8191521
>> - renamed the test LongPath.java to LongBCP.java to better reflect
>> what is being tested;
>> - some comments update on os_windows.cpp
>>
>> updated webrev:
>> http://cr.openjdk.java.net/~ccheung/8188122/webrev.05/
>>
>> thanks,
>> Calvin
>>
>> On 11/9/17, 9:23 AM, Calvin Cheung wrote:
>>> Hi Thomas,
>>>
>>> On 11/8/17, 10:40 PM, Thomas Stüfe wrote:
>>>> Hi Calvin,
>>>>
>>>> On Wed, Nov 8, 2017 at 6:27 PM, Calvin Cheung
>>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>> wrote:
>>>>
>>>>
>>>>
>>>> On 11/7/17, 6:12 AM, Thomas Stüfe wrote:
>>>>> Hi Calvin,
>>>>>
>>>>> On Wed, Nov 1, 2017 at 8:07 PM, Calvin Cheung
>>>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 10/27/17, 12:55 AM, Thomas Stüfe wrote:
>>>>>> Hi Calvin,
>>>>>>
>>>>>> On Fri, Oct 27, 2017 at 2:03 AM, Calvin Cheung
>>>>>> <calvin.cheung at oracle.com
>>>>>> <mailto:calvin.cheung at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Thomas,
>>>>>>
>>>>>> On 10/25/17, 11:54 PM, Thomas Stüfe wrote:
>>>>>>
>>>>>> Hi Calvin,
>>>>>>
>>>>>> this is a worthwhile addition, thank you for your
>>>>>> work!
>>>>>>
>>>>>> Thanks for your review.
>>>>>>
>>>>>>
>>>>>> Thanks for your work :)
>>>>>>
>>>>>> First off, there is another issue in
>>>>>> file_attribute_data_to_stat(). We actually had this issue
>>>>>> before, but your work uncovered it:
>>>>>>
>>>>>> According to POSIX
>>>>>> (http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html
>>>>>>
>>>>>> <http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html>)
>>>>>>
>>>>>> and every unix manpage I looked at (solaris, linux, aix..),
>>>>>> st_ctime is not the file creation time but the last time
>>>>>> file status was changed. Only Microsoft gets this wrong in
>>>>>> their posix layer, its stat() function returns
>>>>>> (https://msdn.microsoft.com/en-us/library/14h5k7ff.aspx
>>>>>> <https://msdn.microsoft.com/en-us/library/14h5k7ff.aspx>)
>>>>>> creation time in st_ctime.
>>>>>>
>>>>>> But as os::stat() is a platform independent layer, I'd say
>>>>>> we should behave the same on all platforms, and that would
>>>>>> be the Posix way.
>>>>>>
>>>>>> I did not find any code calling os::stat() and using
>>>>>> st_ctime, so this is still save to change for windows.
>>>>>> (Note that I found that perfMemory_xxx.cpp uses plain OS
>>>>>> ::stat and misuses ctime as "creation time" - I opened a
>>>>>> bug for that
>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8190260
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8190260> - but
>>>>>> that does not affect us, as they do not call os::stat()).
>>>>>>
>>>>>> There is one more complication: in os::stat() we use plain
>>>>>> ::stat() in the single byte case.:
>>>>>>
>>>>>> *+ if (strlen(path) < MAX_PATH) {*
>>>>>> *+ ret = ::stat(pathbuf, sbuf);*
>>>>>> *+ } else {*
>>>>>> *
>>>>>> *
>>>>>> But ::stat() on Windows is broken, as I wrote above. We
>>>>>> should not use it, especially not if we change the meaing
>>>>>> of st_ctime in the double byte path.
>>>>>>
>>>>>> My suggestion would be to just always call
>>>>>> GetFileAttributes(), also for the single byte path. A very
>>>>>> simple solution would be to just always go the double byte
>>>>>> path with UNC translation, regardless of the path
>>>>>> length. Or, if you are worried about performance, for paths
>>>>>> shorter than MAX_PATH we use GetFileAttributesA(), for
>>>>>> longer paths GetFileAttributesW with UNC translation. In
>>>>>> both cases you get a WIN32_FILE_ATTRIBUTE_DATA which you
>>>>>> can feed tp your file_attribute_data_to_stat() routine and
>>>>>> have the struct stat you return be always consistent.
>>>>> I ran into an issue with the dwFileAttributes value for a
>>>>> jar file. On Windows Server O/S, the value is set to 0x2020
>>>>> which is (FILE_ATTRIBUTE_NOT_CONTENT_INDEXED |
>>>>> FILE_ATTRIBUTE_ARCHIVE) but on a desktop O/S like Windows 7,
>>>>> it is set to 0x0020 which is just FILE_ATTRIBUTE_ARCHIVE.
>>>>> I've fixed it in file_attribute_data_to_stat().
>>>>>
>>>>>
>>>>> Oh.. :( good catch! But that opens a new can of worms I did not
>>>>> see before:
>>>>>
>>>>> FILE_ATTRIBUTE_NORMAL is documented as "A file that does not
>>>>> have other attributes set. This attribute is valid only when
>>>>> used alone." In addition to this flag, we have a multitude of
>>>>> things like FILE_ATTRIBUTE_ARCHIVE, FILE_ATTRIBUTE_ENCRYPTED,
>>>>> FILE_ATTRIBUTE_READONLY ... etc, all cases where we assume this
>>>>> is a normal file in Unix terminology and where we would expect
>>>>> os::stat to return S_IFREG, but where according to the msdn doc
>>>>> FILE_ATTRIBUTE_NORMAL is not set.
>>>>>
>>>>> Looking at what others do in this scenario (Looked at mingw
>>>>> sources and at ReactOS - I am not posting any source code here
>>>>> for legal reasons but feel free to look for yourself), the usual
>>>>> way to translate WIN32_FILE_ATTRIBUTES to struct stat seems to
>>>>> be:
>>>>> "if FILE_ATTRIBUTE_DIRECTORY then set S_IFDIR else S_IFREG" (so,
>>>>> no dependency on FILE_ATTRIBUTE_NORMAL).
>>>> This makes sense but I ran into similar problem as before - the
>>>> dwFileAttributes has a different value on a windows server O/S vs
>>>> desktop O/S. So I need to do the check as follows:
>>>>
>>>> + // A directory has the dwFileAttributes value of 0x2010
>>>> which is
>>>> + // (FILE_ATTRIBUTE_NOT_CONTENT_INDEXED |
>>>> FILE_ATTRIBUTE_ARCHIVE)
>>>> + // on Windows Server O/S but the value is 0x0010 on Windows
>>>> desktop O/S
>>>> + // such as Windows 7.
>>>> + if ((file_data.dwFileAttributes& FILE_ATTRIBUTE_DIRECTORY)
>>>> != 0) {
>>>> + sbuf->st_mode |= S_IFDIR;
>>>> + } else {
>>>> + sbuf->st_mode |= S_IFREG;
>>>> + }
>>>>
>>>> I scratched my head a bit about the comment till I understood that
>>>> you mean "if FILE_ATTRIBUTE_DIRECTORY bit is set it is a directory
>>>> regardless of which other flags are set" instead of "if
>>>> flags==FILE_ATTRIBUTE_DIRECTORY it is a directory". Sure, I guess
>>>> my comment above was sloppy, but this was what I meant. I am not
>>>> even sure the comment is needed, this is quite self-explaining.
>>> I've noticed a typo in the above comment:
>>> + // (FILE_ATTRIBUTE_NOT_CONTENT_INDEXED | FILE_ATTRIBUTE_ARCHIVE)
>>>
>>> FILE_ATTRIBUTE_ARCHIVE should be FILE_ATTRIBUTE_DIRECTORY
>>>
>>> I'll correct it before push.
>>>
>>>>
>>>> updated webrev:
>>>> http://cr.openjdk.java.net/~ccheung/8188122/webrev.04/
>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.04/>
>>>>
>>>>
>>>> I am fine with all the changes now. Thank you for your work and
>>>> patience.
>>> Thanks for your discussions and review.
>>>
>>> thanks,
>>> Calvin
>>>>
>>>> Kind Regards, Thomas
>>>>
>>>>
>>>>
>>>>
>>>> Diff from webrev.03:
>>>>
>>>> < --- old/src/hotspot/os/windows/os_windows.cpp 2017-11-08
>>>> 08:50:40.170786397 -0800
>>>> < +++ new/src/hotspot/os/windows/os_windows.cpp 2017-11-08
>>>> 08:50:39.803751877 -0800
>>>> < @@ -4060,41 +4060,119 @@
>>>> ---
>>>> > --- old/src/hotspot/os/windows/os_windows.cpp 2017-11-01
>>>> 09:40:13.657460425 -0700
>>>> > +++ new/src/hotspot/os/windows/os_windows.cpp 2017-11-01
>>>> 09:40:13.261423192 -0700
>>>> > @@ -4060,41 +4060,121 @@
>>>> 25,29c25
>>>> < + // A directory has the dwFileAttributes value of 0x2010 which is
>>>> < + // (FILE_ATTRIBUTE_NOT_CONTENT_INDEXED | FILE_ATTRIBUTE_ARCHIVE)
>>>> < + // on Windows Server O/S but the value is 0x0010 on Windows
>>>> desktop O/S
>>>> < + // such as Windows 7.
>>>> < + if ((file_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
>>>> != 0) {
>>>> ---
>>>> > + if (file_data.dwFileAttributes == FILE_ATTRIBUTE_DIRECTORY) {
>>>> 31c27,33
>>>> < + } else {
>>>> ---
>>>> > + }
>>>> > + if ((file_data.dwFileAttributes == FILE_ATTRIBUTE_NORMAL) ||
>>>> > + // an archive file such as a jar file has the
>>>> dwFileAttributes value of
>>>> > + // 0x2020 (FILE_ATTRIBUTE_NOT_CONTENT_INDEXED |
>>>> FILE_ATTRIBUTE_ARCHIVE)
>>>> > + // on Windows Server O/S but the value is 0x0020 on
>>>> > + // Windows desktop O/S such as Windows 7.
>>>> > + ((file_data.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE)
>>>> != 0)) {
>>>>
>>>>>
>>>>> I wonder about other special cases which should work too:
>>>>> - read-only files should be S_IFREG and !S_IWRITE,
>>>> For a read-only system file under the user's home dir.
>>>>
>>>> st_mode & 0xFF00 = 0x8100 = S_IFREG | S_IREAD
>>>> dwFileAttributes = 39 = (FILE_ATTRIBUTE_ARCHIVE |
>>>> FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM |
>>>> FILE_ATTRIBUTE_READONLY)
>>>>> read-only directories should be S_IFDIR and !S_IWRITE.
>>>> I've tried the C:\progra~1 dir.
>>>>
>>>> st_mode & 0xFF00 = 0x4100 = S_IFDIR | S_IREAD
>>>> dwFileAttributes = 17 = (FILE_ATTRIBUTE_DIRECTORY |
>>>> FILE_ATTRIBUTE_READONLY)
>>>>> - We should tread the device root ("C:\") as a directory (so,
>>>>> os::stat("c:\") should return S_IFDIR). Does this work?
>>>> This one works too.
>>>>
>>>> st_mode & 0xFF00 = 0x4100 = S_IFDIR | S_IREAD
>>>> dwFileAttributes = 22 = (FILE_ATTRIBUTE_DIRECTORY |
>>>> FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)
>>>>>
>>>>> I've also used GetTickCounts() to measure the performance of
>>>>> ::stat() vs GetFileAttributesA() plus
>>>>> file_attribute_data_to_stat(). There's no difference at the
>>>>> ms resolution. Using the high-resolution timestamp
>>>>> (https://msdn.microsoft.com/en-us/library/windows/desktop/dn553408(v=vs.85).aspx)
>>>>>
>>>>> <https://msdn.microsoft.com/en-us/library/windows/desktop/dn553408%28v=vs.85%29.aspx%29>,
>>>>>
>>>>> it doesn't show much difference.
>>>>>
>>>>>
>>>>> stat() seems to be implemented using FindFirstFile() (see crt
>>>>> sources if you can), and we call GetFileAttributesA(), so I do
>>>>> not think this differs much.
>>>> Yes, I saw the same in my Visual Studio installation.
>>>>
>>>> thanks,
>>>> Calvin
>>>>
>>>>>>
>>>>>> But onward:
>>>>>>
>>>>>>
>>>>>>
>>>>>> =========================
>>>>>>
>>>>>> src/hotspot/os/windows/os_windows.cpp
>>>>>>
>>>>>> Could you please use wchar_t instead of WCHAR?
>>>>>>
>>>>>> Fixed.
>>>>>>
>>>>>>
>>>>>> ---------------
>>>>>> in os::stat():
>>>>>>
>>>>>> This cumbersome malloc/strcpy sequence:
>>>>>>
>>>>>> ! size_t path_len = strlen(path) + 1;
>>>>>> + char* pathbuf = (char*)os::malloc(path_len *
>>>>>> sizeof(char), mtInternal);
>>>>>> + if (pathbuf == NULL) {
>>>>>> + errno = ENOMEM;
>>>>>> return -1;
>>>>>> }
>>>>>> os::native_path(strcpy(pathbuf, path));
>>>>>>
>>>>>> can be reduced to a simple strdup:
>>>>>>
>>>>>> char* pathbuf = os::strdup(path, mtInternal);
>>>>>> if (pathbuf == NULL) {
>>>>>> errno = ENOMEM;
>>>>>> return -1;
>>>>>> }
>>>>>> os::native_path(pathbuf);
>>>>>>
>>>>>> This also would separate strcpy() from
>>>>>> os::native_path() which is a bit unreadable.
>>>>>>
>>>>>> I've made the change.
>>>>>>
>>>>>>
>>>>>> --------------------
>>>>>>
>>>>>> The single-byte path (strdup, ::stat()), together
>>>>>> with its free(), can be moved inside the
>>>>>> (strlen(path) < MAX_PATH) condition.
>>>>>> os::native_path will not change the path length (it
>>>>>> better not, as it operates on the input buffer).
>>>>>> That avoids having two allocations on the
>>>>>> doublebyte path.
>>>>>>
>>>>>>
>>>>>> os::native_path() converts a path like C:\\\\somedir to
>>>>>> C:\\somedir
>>>>>> So I'll need the converted path in the wchar_t case
>>>>>> too. The freeing of the pathbuf is being done at the
>>>>>> end of the function or in the middle of the wchar_t
>>>>>> case if there's an error.
>>>>>>
>>>>>>
>>>>>> Oh, you are right,of course. I missed that it this is
>>>>>> needed for both paths.
>>>>>>
>>>>>>
>>>>>>
>>>>>> -----------------------
>>>>>>
>>>>>> But seeing that translation to UNC paths is
>>>>>> something we might want more often, I would combine
>>>>>> allocation and UNC prefix adding to one function
>>>>>> like this, to avoid the memove and increase
>>>>>> reusability:
>>>>>>
>>>>>> // Creates an unc path from a single byte path.
>>>>>> Return buffer is allocated in C heap and needs to
>>>>>> be freed by caller. Returns NULL on error.
>>>>>> static whchar_t* create_unc_path(const char* s) {
>>>>>> - does s start with \\?\ ?
>>>>>> - yes:
>>>>>> - os::malloc(strlen(s) + 1) and
>>>>>> mbstowcs_s
>>>>>> - no:
>>>>>> - os::malloc(strlen(s) + 1 + 4),
>>>>>> mbstowcs_s to fourth position in string, prefix
>>>>>> with L"\\?\"
>>>>>> }
>>>>>>
>>>>>>
>>>>>> I also include the case for adding L"\\\\?\\UNC\0" at
>>>>>> the beginning to be consistent with
>>>>>> libjava/canonicalize_md.c.
>>>>>>
>>>>>> We also need error checking to mbstowcs_s.
>>>>>>
>>>>>> I've added assert like the following after the call:
>>>>>>
>>>>>> assert(converted_chars == path_len, "sanity");
>>>>>>
>>>>>>
>>>>>>
>>>>>> create_unc_path() :
>>>>>>
>>>>>> - could you convert the /**/ to // comments, please?
>>>>> Fixed.
>>>>>
>>>>>
>>>>> thanks.
>>>>>
>>>>>
>>>>>>
>>>>>> - not sure about the assert after mbstowcs_s: if we happen
>>>>>> to encounter an invalid multibyte character, function will
>>>>>> fail and return an error:
>>>>>>
>>>>>> https://msdn.microsoft.com/en-us/library/eyktyxsx.aspx
>>>>>> <https://msdn.microsoft.com/en-us/library/eyktyxsx.aspx>
>>>>>> "If mbstowcs_s encounters an invalid multibyte character,
>>>>>> it puts 0 in *``pReturnValue, sets the destination buffer
>>>>>> to an empty string, sets errno to EILSEQ, and returns
>>>>>> EILSEQ."
>>>>> I've changed create_unc_path() so that the caller will get
>>>>> the errno and removed the assert.
>>>>>
>>>>> static wchar_t* create_unc_path(const char* path, errno_t
>>>>> &err)
>>>>>
>>>>>
>>>>> Okay, works for me.
>>>>>
>>>>>
>>>>>>
>>>>>> As this is dependent on user data, we should not assert,
>>>>>> but handle the return code of mbstowcs_s gracefully. Same
>>>>>> goes for libjava/canonicalize_md.c.
>>>>>>
>>>>>> - Here: ::mbstowcs_s(&converted_chars, &wpath[7], path_len
>>>>>> + 7, path, path_len);
>>>>>> third parameter is wrong, as we hand in an offset into the
>>>>>> buffer, we must decrement the buffer size by this offset,
>>>>>> so correct would be path_len +7 - 7 or just path_len.
>>>>>>
>>>>>> - Same error below: + ::mbstowcs_s(&converted_chars,
>>>>>> &wpath[4], path_len + 4, path, path_len);
>>>>> Fixed in both places.
>>>>>
>>>>>
>>>>> Okay.
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> Just for cleanliness, I would then wrap
>>>>>> deallocation into an own function.
>>>>>>
>>>>>> static viud destroy_unc_path(whchar_t* s) {
>>>>>> os::free(s); }
>>>>>>
>>>>>> I've added the destroy function.
>>>>>>
>>>>>>
>>>>>> These two functions could be candidates of putting
>>>>>> into os::win32 namespace, as this form of ANSI->UNC
>>>>>> path translation is quite common - whoever wants to
>>>>>> use the xxxxW() functions must do this.
>>>>>>
>>>>>> I'm leaving them in os_windows.cpp since they're being
>>>>>> used only within that file.
>>>>>>
>>>>>>
>>>>>> Fine by me.
>>>>>>
>>>>>>
>>>>>>
>>>>>> -----------------------
>>>>>>
>>>>>> FindFirstFileW:
>>>>>>
>>>>>> I am pretty sure that you can achieve the same
>>>>>> result with GetFileAttributesExW(). It also returns
>>>>>> WIN32_FIND_DATAW.
>>>>>>
>>>>>> It actually returns WIN32_FILE_ATTRIBUTE_DATA and is
>>>>>> very similar to WIN32_FIND_DATAW.
>>>>>>
>>>>>>
>>>>>> It is way more straightforward to use than
>>>>>> FindFirstFileW, as it does not require you to write
>>>>>> a callback hook.
>>>>>>
>>>>>> I've switched to using GetFileAttributesExW().
>>>>>>
>>>>>>
>>>>>> Thank you, this is way more readable.
>>>>>> Another issue: do we still need the fix for 6539723 if we
>>>>>> switch from stat() to GetFileAttributesExW()? This fix
>>>>>> looks important, but the comment
>>>>>> indicates that it could break things if the original bug is
>>>>>> not present.
>>>>>>
>>>>>> Btw, this is another strong argument for scrapping ::stat()
>>>>>> altogether on all code paths, not only for long input
>>>>>> paths, because stat() and GetFileAttributesExW() may behave
>>>>>> differently. So it would be good to use the same API on all
>>>>>> code paths, in order to get the best test coverage.
>>>>> For this round of change, I'm using
>>>>> GetFileAttributesEx[A|W]() for both code paths.
>>>>>
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~ccheung/8188122/webrev.03/
>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.03/>
>>>>>
>>>>> thanks,
>>>>> Calvin
>>>>>
>>>>>
>>>>> Okay, all good apart from the issues mentioned above. Thanks for
>>>>> your work!
>>>>>
>>>>> Best Regards, Thomas
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> -------
>>>>>>
>>>>>> eval_find_data(): This is more of a generic helper
>>>>>> function, could you rename this to something
>>>>>> clearer, e.g. make_double_word() ?
>>>>>>
>>>>>> Ok. I've renamed it.
>>>>>>
>>>>>>
>>>>>> Also, setup_stat(), could this be renamed more
>>>>>> clearly to something like WIN32_FIND_DATA_to_stat?
>>>>>> or lowercase if this bothers you :)
>>>>>>
>>>>>> I'm naming the function as
>>>>>> file_attribute_data_to_stat() to match with the data
>>>>>> structure name.
>>>>>>
>>>>>>
>>>>>> Thanks for taking my suggestions.
>>>>>>
>>>>>>
>>>>>>
>>>>>> ==================================
>>>>>> src/hotspot/share/classfile/classLoader.cpp
>>>>>>
>>>>>> In ClassPathDirEntry::open_stream(), I would feel
>>>>>> better if we were asserting _dir and name to be !=
>>>>>> NULL before feeding it to strlen.
>>>>>>
>>>>>> I've added an assert statement.
>>>>>>
>>>>>>
>>>>>> ===================
>>>>>>
>>>>>> Here's an updated webrev:
>>>>>> http://cr.openjdk.java.net/~ccheung/8188122/webrev.02/
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.02/>
>>>>>>
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>> thanks,
>>>>>> Calvin
>>>>>>
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Oct 25, 2017 at 9:48 PM, Calvin Cheung
>>>>>> <calvin.cheung at oracle.com
>>>>>> <mailto:calvin.cheung at oracle.com>
>>>>>> <mailto:calvin.cheung at oracle.com
>>>>>> <mailto:calvin.cheung at oracle.com>>> wrote:
>>>>>>
>>>>>> I've reworked this fix by using the Unicode
>>>>>> version of open
>>>>>> (wopen) to handle path name longer than max
>>>>>> path with the path
>>>>>> prefixed to indicate an extended length path as
>>>>>> described in [1].
>>>>>>
>>>>>> The Unicode version of stat (wstat) doesn't
>>>>>> work well with long
>>>>>> path [2]. So FindFirstFileW will be used.The
>>>>>> data in
>>>>>> WIN32_FIND_DATA returned from FindFirstFileW
>>>>>> needs to be
>>>>>> transferred to the stat structure since the
>>>>>> caller expects a
>>>>>> return stat structure and other platforms
>>>>>> return a stat structure.
>>>>>>
>>>>>> In classLoader.cpp, calculate the size of
>>>>>> buffer required instead
>>>>>> of limiting it to JVM_MAXPATHLEN.
>>>>>> In os_windows.cpp, dynamically allocate buffers
>>>>>> in os::open and
>>>>>> os::stat.
>>>>>>
>>>>>> updated webrev:
>>>>>> http://cr.openjdk.java.net/~ccheung/8188122/webrev.01/
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.01/>
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.01/
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.01/>>
>>>>>>
>>>>>> It passed hs-tier2 testing using mach5.
>>>>>>
>>>>>> thanks,
>>>>>> Calvin
>>>>>>
>>>>>> [1]
>>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#MAX_PATH
>>>>>>
>>>>>> <https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx#MAX_PATH>
>>>>>>
>>>>>> <https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx#MAX_PATH
>>>>>>
>>>>>> <https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx#MAX_PATH>>
>>>>>>
>>>>>>
>>>>>> [2]
>>>>>> https://social.msdn.microsoft.com/Forums/vstudio/en-US/3c093ea9-f0aa-446d-b648-2dabe8480430/stat-and-long-names?forum=vcgeneral
>>>>>>
>>>>>> <https://social.msdn.microsoft.com/Forums/vstudio/en-US/3c093ea9-f0aa-446d-b648-2dabe8480430/stat-and-long-names?forum=vcgeneral>
>>>>>>
>>>>>> <https://social.msdn.microsoft.com/Forums/vstudio/en-US/3c093ea9-f0aa-446d-b648-2dabe8480430/stat-and-long-names?forum=vcgeneral
>>>>>>
>>>>>> <https://social.msdn.microsoft.com/Forums/vstudio/en-US/3c093ea9-f0aa-446d-b648-2dabe8480430/stat-and-long-names?forum=vcgeneral>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 10/16/17, 3:15 PM, Calvin Cheung wrote:
>>>>>>
>>>>>> JBS:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8188122
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8188122>
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8188122
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8188122>>
>>>>>>
>>>>>> Adding a warning message if the full path
>>>>>> or the directory
>>>>>> length exceeds MAX_PATH on windows.
>>>>>>
>>>>>> Example warning messages.
>>>>>>
>>>>>> 1) The full path exceeds MAX_PATH:
>>>>>>
>>>>>> Java HotSpot(TM) 64-Bit Server VM warning:
>>>>>> construct full path
>>>>>> name failed: total length 270 exceeds max
>>>>>> length of 260
>>>>>> dir
>>>>>> T:\\testoutput\\jtreg\\JTwork\\classes\\2\\runtime\\LoadClass\\LongPath.d\\xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>>>>>>
>>>>>> length 259
>>>>>> name Hello.class length 11
>>>>>>
>>>>>> 2) The directory path exceeds MAX_PATH:
>>>>>>
>>>>>> Java HotSpot(TM) 64-Bit Server VM warning:
>>>>>> os::stat failed:
>>>>>> path length 265 exceeds max length 260
>>>>>> path
>>>>>> T:\\testoutput\\jtreg\\JTwork\\classes\\2\\runtime\\LoadClass\\LongPath.d\\xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\\xxxxx
>>>>>>
>>>>>>
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~ccheung/8188122/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.00/>
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Eccheung/8188122/webrev.00/>>
>>>>>>
>>>>>> Testing:
>>>>>> JPRT (including the new test)
>>>>>>
>>>>>> thanks,
>>>>>> Calvin
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>
More information about the hotspot-runtime-dev
mailing list