SECOND CALL: code review for jvmstat/jps fix (6954420)

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Feb 15 18:54:25 PST 2011


Karen and Poonam,

Two reviewers commenting on the same line a few minutes
apart... the universe speaks... and I listen...

I changed to:

1361c1361,1362
<         warning("could not stat file %s: %d\n", filename, ret_code);
---
 >         warning("Could not get status information from file %s: %s\n",
 >             filename, strerror(errno));

Thanks for the reviews!

Dan


On 2/15/2011 7:12 PM, Karen Kinnear wrote:
> Dan,
>
> Approved.
>
> Looks very tricky to track down.
>
> Only minor suggestion - line 1361 warning: if ret_code == OS_ERR, it 
> might be nice to
> print %s and strerror(errno) rather than %d and ret_code. Not a big deal.
>
> thanks,
> Karen

> On 2/15/2011 7:26 PM, Poonam Bajaj wrote:
>> Looks good. One minor suggestion - you may consider changing the 
>> following warning message to something like "Could not get status 
>> information on file ".
>>
>> 1357     struct stat statbuf;
>> 1358     int ret_code = ::stat(filename, &statbuf);
>> 1359     if (ret_code == OS_ERR) {
>> 1360       if (PrintMiscellaneous && Verbose) {
>> 1361         warning("could not stat file %s: %d\n", filename, 
>> ret_code);
>> 1362       }
>>
>>
>> Thanks,
>> Poonam



More information about the hotspot-runtime-dev mailing list