RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework

[Ivan Gerasimov]

An alternative, more compact variant might be

------------------------
     String getErrorMessage(InputStream sis, int maxlen) throws 
IOException {
         int n, off = 0, len = maxlen + 1;
         byte b[] = new byte[len];
         while ((n = sis.read(b, off, len - off)) > 0)
             off += n;
         return (off == 0) ? null
                 : (off < len)
                 ? new String(b, 0, off, "UTF-8")
                 : new String(b, 0, maxlen, "UTF-8") + " ...";
     }
------------------------

Not a big deal, of course.

Sincerely yours,
Ivan



On 04.04.2014 16:24, Ivan Gerasimov wrote:
> Now you reintroduced the smallish issue, when the message is exactly 
> 200 characters (or whatever maxlen is).
> The dots will be appended to the message, even though it's not necessary.
>
> I think the only reliable way to deal with it is to try to read one 
> extra character from sis.
>
> Something like this should do:
> ------------------------
>     String getErrorMessage(InputStream sis, int maxlen) throws 
> IOException {
>         byte b[] = new byte[maxlen + 1];
>         int n, off = 0, len = b.length;
>         do {
>             n = sis.read(b, off, len);
>             if (n == -1) {
>                 break;
>             }
>             off += n;
>             len -= n;
>         } while (off < maxlen);
>
>         String message = null;
>         if (off > 0) {
>             message = (off > maxlen)
>                     ? new String(b, 0, maxlen, "UTF-8") + " ..."
>                     : new String(b, 0, off, "UTF-8");
>         }
>         return message;
>     }
> ------------------------
>
> Sincerely yours,
> Ivan
>
> On 04.04.2014 16:08, Staffan Larsen wrote:
>> I’m afraid you are right! Doh. Need to add more testing...
>>
>> How about this change:
>>
>> --- a/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java
>> +++ b/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java
>> @@ -267,9 +267,11 @@
>>       String getErrorMessage(InputStream sis, int maxlen) throws 
>> IOException {
>>           byte b[] = new byte[maxlen];
>>           int n, off = 0, len = maxlen;
>> +        boolean complete = false;
>>           do {
>>               n = sis.read(b, off, len);
>>               if (n == -1) {
>> +                complete = true;
>>                   break;
>>               }
>>               off += n;
>> @@ -280,7 +282,7 @@
>>           if (off > 0) {
>>               message = new String(b, 0, off, "UTF-8");
>>           }
>> -        if (off > b.length && message != null) {
>> +        if (!complete && message != null) {
>>               message += " ...";
>>           }
>>           return message;
>>
>>
>> On 4 apr 2014, at 13:55, Ivan Gerasimov <ivan.gerasimov at oracle.com> 
>> wrote:
>>
>>> Thank you Staffan for fixing them!
>>>
>>> But I'm afraid that now the function will never add ellipsis to the 
>>> message, even if it gets truncated.
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>> On 04.04.2014 15:47, Staffan Larsen wrote:
>>>> Thanks for finding these bugs, Ivan!
>>>>
>>>> I have updated the webrev at: 
>>>> http://cr.openjdk.java.net/~sla/8039173/webrev.01/, and I have also 
>>>> included the diff below.
>>>>
>>>> The updated webrev also has some changes in the javadoc for 
>>>> VirtualMachine to clarify that some methods can now throw 
>>>> AttachOperationFailedException.
>>>>
>>>> Thanks,
>>>> /Staffan
>>>>
>>>>
>>>> diff --git 
>>>> a/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java 
>>>> b/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java
>>>> --- a/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java
>>>> +++ b/src/share/classes/sun/tools/attach/HotSpotVirtualMachine.java
>>>> @@ -266,18 +266,21 @@
>>>>        */
>>>>       String getErrorMessage(InputStream sis, int maxlen) throws 
>>>> IOException {
>>>>           byte b[] = new byte[maxlen];
>>>> -        int n, off = 0, len = b.length;
>>>> +        int n, off = 0, len = maxlen;
>>>>           do {
>>>>               n = sis.read(b, off, len);
>>>> +            if (n == -1) {
>>>> +                break;
>>>> +            }
>>>>               off += n;
>>>>               len -= n;
>>>> -        } while (n >= 0 && off < b.length);
>>>> +        } while (off < maxlen);
>>>>
>>>>           String message = null;
>>>>           if (off > 0) {
>>>>               message = new String(b, 0, off, "UTF-8");
>>>>           }
>>>> -        if (off == b.length && message != null) {
>>>> +        if (off > b.length && message != null) {
>>>>               message += " ...";
>>>>           }
>>>>           return message;
>>>>
>>>>
>>>> On 4 apr 2014, at 11:18, Ivan Gerasimov <ivan.gerasimov at oracle.com> 
>>>> wrote:
>>>>
>>>>> Hi Staffan!
>>>>>
>>>>> I think there is a couple of minor bugs in 
>>>>> getErrorMessage(InputStream sis, int maxlen).
>>>>>
>>>>> 1) If maxlen is exactly the size of the message to read, the 
>>>>> function will add an ellipsis, even though the message isn't 
>>>>> truncated,
>>>>> 2) If maxlen is greater than needed, then sis.read(b, off, len) at 
>>>>> the line #271 will eventually return -1, and it will cause the 
>>>>> message to lose its last character.
>>>>>
>>>>> Sincerely yours,
>>>>> Ivan
>>>>>
>>>>
>>
>>
>
>
>