Thank you for the comments, both Dmitry and Mikael. Replies inline.
Modified webrev: http://cr.openjdk.java.net/~rprotacio/8138916.01/
(Retested and works fine.)
On 10/27/2015 7:52 AM, Dmitry Samersoff wrote:
Rachel,
In worst case (windows, overflow) the code would parse format string
four times.
You're right. I've added a check. New amount of times vnsprintf() is called:
posix, fits => 1
posix, overflow => 2
windows, fits => 1
windows, overflow => 3 (the best performance we can get)
Also NEW_C_HEAP_ARRAY takes a lock and might become a
performance bottleneck.
POSIX equivalent of _vscprintf(fmt, args) is vsnprintf(NULL, 0, fmt, args)
So it might be better to create os::log_vcprintf(fmt, args), then write
code at log.hpp as:
int buf_len = os::log_vcprintf(fmt, args);
assert(buf_len < 2*page_size, "Stack overflow in logging");
char buf[buf_len];
vsprintf(buf, fmt, args);
I don't think this would work anyway. A char[] needs a static size.
Thanks,
Rachel
-Dmitry
On 2015-10-26 23:58, Rachel Protacio wrote:
Hello, all,
Please review this fix to the faulty write function from the Unified
Logging framework.
Summary: In logging/log.hpp, the logging vwrite function previously
asserted that the buffer remains within 512 characters, which is too
short for logging message of non-pre-determined length, e.g. for vtable
and itable function names. Now, the function resizes the buffer to the
appropriate length, asserting only that the resulting buffer is valid.
Includes tag "logtest" to test that logging can print an output of 518
characters.
Open webrev:http://cr.openjdk.java.net/~rprotacio/8138916/
Bug:https://bugs.openjdk.java.net/browse/JDK-8138916
Includes own acceptance test, and passes JPRT and remote hotspot/test
tests.
Thank you,
Rachel
