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

[Dmitry Samersoff]

Staffan,

Looks good for me.

Could you comment in bug report that this changeset also fixes incorrect
usage of off parameter in *VirtualMachine_read functions.


-Dmitry




On 2014-04-29 18:08, Staffan Larsen wrote:
> Thanks for the comments. I have updated the review with both.
> 
> webrev: http://cr.openjdk.java.net/~sla/8039173/webrev.05/
> 
> I do not actually have any Reviewers for this change. Anyone willing to take it on is much appreciated.
> 
> Thanks,
> /Staffan
> 
> 
> On 29 apr 2014, at 15:22, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
>> On 16/04/2014 10:21, Staffan Larsen wrote:
>>> On 14 apr 2014, at 17:17, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>>
>>>> :
>>>> For someone looking at the Virtualmachine API then I don't think the javadoc is clear enough to understand when one might get the specific AttachOperationFailedException vs. the more general IOException. I think it means that there was communication with the target VM but that the operation failed for some reason but I don't think this will be obvious to the reader.
>>> I have tried to clarify the wording in the javadoc. Suggestions for improvements are welcome.
>> Sorry for the delay, I was on away for a few days and just catching up with this again.
>>
>> The updated descriptions looks much better. For IOException then it might be better to have a bit of wriggle room to allow for other I/O errors that might not be communication related. So maybe something like "If an I/O error occurs, a communication error for example, that cannot be identified as an error to indicate that the operation failed in the target VM".
>>
>>> :
>>>> For the new exception then it would be good to add @since and also a copyright header.
>>> Fixed.
>>>
>> Thanks, a formatting nit at L42, I assume that the "{" will fit at the end of line 41.
>>
>> I don't have cycles at the moment to go through the implementation changes but I think you have other reviewers for that.
>>
>> -Alan.
>>
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.