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

[Dmitry Samersoff]

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* lookup_diagnosticArgumentEnum(const char* field_name, oop object)
>> and
>> 52: void fill_in_parser(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
* There will come soft rains ...