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> 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/ > 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 > > >
