On Wed, 11 Sep 2024 13:32:49 GMT, Simon Tooke <sto...@openjdk.org> wrote:
>> This is a port of [JDK-8318636](https://github.com/openjdk/jdk/pull/16301) >> to Windows. >> >> System.map and System.dump_map are implemented using the Windows API and >> provide roughly the same information in the same format. Most of the heavy >> lifting was implemented by @tstuefe in #16301 - this PR adds the Windows >> implementation and enables the common code for Windows 64 bit. >> >> [Sample output (with NMT >> enabled)](https://github.com/user-attachments/files/16663332/vm_memory_map_760.txt) > > Simon Tooke has updated the pull request incrementally with two additional > commits since the last revision: > > - prevent VM crash on invalid jcmd > - changes from review Hi Simon, Thanks! Looks good. Just had a question about the empty filename issue. I don’t think there is a case where a filename parameter should accept an empty string (but I might be wrong). If that’s the case, I wonder if it would be better to just deal with all empty filename parameter errors in the dcmd argument parser. For example, in [diagnosticArgument.cpp](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/services/diagnosticArgument.cpp#L186 ). Perhaps, we could have a check like this: if (strcmp(type(), "FILE") == 0) { if (str == NULL || *str == 0) { stringStream error_msg; error_msg.print("Filename is empty or not specified. %s", str); THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), error_msg.base()); } _value = REALLOC_C_HEAP_ARRAY(char, _value, JVM_MAXPATHLEN, mtInternal); ... } I haven't tested this but this would probably be better handled in a separate RFE though. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20597#issuecomment-2343796609
