JDK-8228402: chdir() and write() errors are not handled in jpackage

[Alexander Matveev]

Hi Andy,

I think trowing exception should be better. We do it in several other 
places, not sure how well our code handles it.
I think it might be good idea to file follow up task to cleanup/improve 
error handling in out native code.
For example PosixProcess::Execute() will return false/true, throw 
exception and also terminates execution in some cases.

http://cr.openjdk.java.net/~herrick/8228402/webrev.02/
Looks fine.

Thanks,
Alexander

On 7/24/2019 5:18 AM, Andy Herrick wrote:
> I used assert because:
>
> 1.) I don't think it could ever happen.
>
> 2.) If it did happen it might be easier to debug with the assert than 
> without it
>
> 3.) I thought it would be difficult to propagate the error.
>
> but perhaps (3) is not true. would it be better (ignoring types for 
> now) to:
>
>>     len = write(FInputHandle, Value.data(), Value.size());
>>
>>     if (len != Value.size()) {
>>
>>             throw Exception(_T("Internal Error - write failed"));
>>
>>     }
>>
> /Andy
>
>
> On 7/23/2019 11:21 PM, Alexander Matveev wrote:
>> Hi Andy,
>>
>> I think it is better not to use assert() in this case since it 
>> terminates app abnormally.
>> Do we really should terminate execution if write() fails in this 
>> case? Can we ignore error?
>> It might be better to handle it gracefully and then return from 
>> main() with error if we cannot ignore error.
>>
>> Thanks,
>> Alexander
>>
>> On 7/23/2019 2:07 PM, Andy Herrick wrote:
>>> Please review the jpackage fix for bug [1] at [2].
>>>
>>> This is a fix for the JDK-8200758-branch branch of the open sandbox 
>>> repository (jpackage).
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8228402
>>>
>>> [2] http://cr.openjdk.java.net/~herrick/8228402/
>>>
>>> /Andy
>>>
>>