Hi Martin,

Thanks, I'll move the line up to be consistent with the local style.

Roger


On 3/20/2015 6:23 PM, Martin Buchholz wrote:
I probably implemented it the way it is intentionally.  I would leave it.

Pedantic changes such as this one won't make any users happy, but we're doing this elsewhere like in java.util.concurrent so I guess it's OK.

But there's a much simpler way to implement it.

Just move

long remainingNanos = unit.toNanos(timeout);
to the beginning of the method body, as we do in java.util.concurrent.

On Fri, Mar 20, 2015 at 2:48 PM, Roger Riggs <roger.ri...@oracle.com <mailto:roger.ri...@oracle.com>> wrote:

    Please review this change to Process.waitFor(timeout, TimeUnit)
    to check for TimeUnit == null before checking for exited = true
    and timeout <= 0.
    The current implementation does not throw NPE if the timeout is <=
    0 or the Process has already
    exited.  The check for the validity of the arguments should come
    before the other conditions.
    This cleanup of the implementation was requested by the JCK team.

    Webrev:
         ://cr.openjdk.java.net/~rriggs/webrev-npe-8067796/
    <http://cr.openjdk.java.net/%7Erriggs/webrev-npe-8067796/>
    Issue:
       8067796: (process) Process.waitFor(timeout, unit) doesn't throw
    NPE if timeout is less than, or equal to zero when unit == null

    Thanks, Roger

    [1] https://bugs.openjdk.java.net/browse/JDK-8067796




Reply via email to