RFR (and sponsor): 7148488: Whitebox tests for the Diagnostic Framework Parser (original) (raw)

Dmitry Samersoff Dmitry.Samersoff at oracle.com
Wed Mar 21 04:31:57 PDT 2012

Nils,

66 void GenDCmdArgument::to_string(StringArrayArgument* f, char* buf, size_t len) {

  1. It seems to me if array doesn't fit to buf we have spare coma at the end. Is it intentional?

  2. We don't check whether we have enough room for comma in a buf. So we can overflow it.

So I would recommend to reformat it something like below:

 int i = 0;
char *cpos = buf,*epos = buf+len;

while(i < array_len){
    const char *next_str = f->array()->at(i);
    int next_len = strlen(next_str);

    if (cpos+next_len+1 > epos){
        break;
    }

    memcpy(cpos,next_str,next_len);
    *(cpos+next_len) = ',';
    cpos+=(next_len+1);
}

epos = (cpos == buf) ? cpos : cpos-1;
*epos = 0;

-Dmitry

On 2012-03-21 14:10, Nils Loodin wrote:

Good idea.

Refereshed this based on your comments and those of Staffan Larsen. New webrev is at http://cr.openjdk.java.net/~nloodin/7148488/webrev.01/ Need one more reviewer-status reviewer then! Regards, Nils Loodin On Mar 16, 2012, at 16:31 , Mikael Gerdin wrote:

Hi,

just a small nitpick in parserTests.cpp: 41: const char* lookupdiagnosticArgumentEnum(const char* fieldname, oop object) and 52: void fillinparser(DCmdParser* parser, oop argument) should probably be static (in the C sense). Otherwise it looks good. /Mikael

On 2012-03-15 13:45, Nils Loodin wrote: Hey all! Here's an implementation of a nice way of doing parser testing from a jtreg-test, through the whitebox testing framework. This patch makes it easy to do parser testing (which will be necessary if we want to change it with any sort of confidence in the future) and partly to show off what can be possible to do with the whitebox testing api. In the added JTREG test, parser testing now works like this from java: //test that we can parse without exceptions wb.parseCommandLine("myIntArgument=10", args); //test that the value get's parsed to what we want parse("myIntArgument", "10", "myIntArgument=10", args); //test that illegal command lines gives exception and aren't silently broken shouldFail("myLongArgument=12m", args); //<-- should fail, doesn't http://cr.openjdk.java.net/~nloodin/7148488/webrev.00/ Regards, Nils Loodin

-- Dmitry Samersoff Java Hotspot development team, SPB04

More information about the serviceability-dev mailing list