Hi Thomas, thanks for the review.
> Can you please add a comment about the new parameter? E.g. "optional > outputStream to which progress- and error messages will be written". Will do. > - in HeapDumper::dump(): while you are on it could you please initialize > my_path to NULL at function start? Do you mean the HeapDumper::dump_heap() method? That seems unrelated to my change. And I would prefer to get a warning about a potentially uninitialized use. > You can actually get rid of HeapDumper::error_as_C_string() altogher now. I thought about it, but the message is more than just the error text (you would get the "Dumping heap to <file> .." part) and is multiple lines. This seems not fitting for an exception message. > Did you test that no jtreg tests fall over the changed output? e.g. > test/jdk/sun/tools/jmap/BasicJMapTest.java, or whatever is under > hotspot/jtreg/serviceability/jcmd I've checked them. They work because they just check that "Heap dump file created" is contained in the output, which is still the case or don't check the output at all (the dcmd tests). Best regards, Ralf
