Rachel,
> I don't think this would work anyway. A char[] needs a static size.
It does work. C++ allows this way of on-stack allocation, C not.
see attached testcase
-Dmitry
On 2015-10-28 17:48, Rachel Protacio wrote:
> 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
>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
CXXFLAGS=-gdwarf-2 -Wall -Werror
test: test.cxx
$(CXX) $(CXXFLAGS) -o $@ $<
test.lst: test.cxx
$(CXX) -Wa,-adhln $(CXXFLAGS) -o /dev/null $<
run:
MALLOC_TRACE=test.log ./test
clean:
rm -f test test.log
#include <mcheck.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
int main(void) {
// Don't forget to setenv MALLOC_TRACE test.log
mtrace();
const char fmt[] = "%s - %024d";
int buf_len = snprintf(NULL, 0, fmt, "bla", 999);
char buf[buf_len];
sprintf(buf, fmt, "bla", 999);
printf("%d %s\n", buf_len, buf);
return 0;
}
