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