code review for jps fix (6637230)

[Daniel D. Daugherty]

Dmitry,

Thanks for the quick review!


On 2/12/2011 2:38 AM, Dmitry Samersoff wrote:
> Dan,
>
> Looks OK for me.

Thanks!


> +
> +                    // leading blank not needed here because of the
> +                    // next append() call
> +                    errorString = "-- main class information 
> unavailable";
> +                    output.append(" ");
> +                    output.append(MonitoredVmUtil.mainClass(vm,
> +                            arguments.showLongPaths()));
>
> May be it could be done as (below)
> to keep errorString uniform:
>
> errorString = "  -- main class information
> String s = MonitoredVmUtil.mainClass(vm,
>                                 arguments.showLongPaths());
> output.append(" ").append(s);

So the trade is the comment and one less blank for
a new temporary variable that's only used here. Hmmm...
I think I'll stick with what's in the current webrev,
but I'll keep that technique more in mind for the next
time I'm whacking diagnostic output...

Dan



>
> -Dmitry
>
>
> On 2011-02-12 05:31, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I have a fix for the following jps bug:
>>
>> 6637230: 2/3 jps doesn't work for application waiting for direct attach
>> Summary: Properly handle exceptions thrown when querying a monitored VM.
>> Reviewed-by:
>>
>> Here is the URL for the webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/6637230-webrev/0/
>>
>> Thanks, in advance, for any reviews.
>>
>> Dan
>>
>
>